diff --git a/CHANGELOG.md b/CHANGELOG.md index be51531d..b29ad2f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. ## Improvements - Django 3.1 -- Handle status callbacks from Twilio, show SMS delivery failures in Integrations +- Handle status callbacks from Twilio, show delivery failures in Integrations ## v1.16.0 - 2020-08-04 diff --git a/hc/api/tests/test_notification_status.py b/hc/api/tests/test_notification_status.py index b58052cd..25128681 100644 --- a/hc/api/tests/test_notification_status.py +++ b/hc/api/tests/test_notification_status.py @@ -89,3 +89,14 @@ class NotificationStatusTestCase(BaseTestCase): self.channel.refresh_from_db() self.assertEqual(self.channel.last_error, "Received complaint.") self.assertFalse(self.channel.email_verified) + + def test_it_handles_twilio_call_status_failed(self): + r = self.client.post(self.url, {"CallStatus": "failed"}) + self.assertEqual(r.status_code, 200) + + self.n.refresh_from_db() + self.assertEqual(self.n.error, "Delivery failed (status=failed).") + + self.channel.refresh_from_db() + self.assertEqual(self.channel.last_error, "Delivery failed (status=failed).") + self.assertTrue(self.channel.email_verified) diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index d0a3c82c..7899e956 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -716,12 +716,15 @@ class NotifyTestCase(BaseTestCase): mock_post.return_value.status_code = 200 self.channel.notify(self.check) - self.assertEqual(Notification.objects.count(), 1) args, kwargs = mock_post.call_args payload = kwargs["data"] self.assertEqual(payload["To"], "whatsapp:+1234567890") + n = Notification.objects.get() + callback_path = f"/api/v1/notifications/{n.code}/status" + self.assertTrue(payload["StatusCallback"].endswith(callback_path)) + # sent SMS counter should go up self.profile.refresh_from_db() self.assertEqual(self.profile.sms_sent, 1) @@ -773,12 +776,15 @@ class NotifyTestCase(BaseTestCase): 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") + n = Notification.objects.get() + callback_path = f"/api/v1/notifications/{n.code}/status" + self.assertTrue(payload["StatusCallback"].endswith(callback_path)) + @patch("hc.api.transports.requests.request") def test_call_limit(self, mock_post): # At limit already: diff --git a/hc/api/transports.py b/hc/api/transports.py index 81833d32..368debbf 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -500,6 +500,7 @@ class Call(HttpTransport): "From": settings.TWILIO_FROM, "To": self.channel.phone_number, "Twiml": twiml, + "StatusCallback": check.status_url, } return self.post(url, data=data, auth=auth) @@ -528,6 +529,7 @@ class WhatsApp(HttpTransport): "From": "whatsapp:%s" % settings.TWILIO_FROM, "To": "whatsapp:%s" % self.channel.phone_number, "Body": text, + "StatusCallback": check.status_url, } return self.post(url, data=data, auth=auth) diff --git a/hc/api/views.py b/hc/api/views.py index ea6677b3..a909f611 100644 --- a/hc/api/views.py +++ b/hc/api/views.py @@ -429,16 +429,20 @@ def notification_status(request, code): error, mark_not_verified = None, False - # Look for "error" and "unsub" keys: + # Look for "error" and "mark_not_verified" keys: if request.POST.get("error"): error = request.POST["error"][:200] mark_not_verified = request.POST.get("mark_not_verified") - # Handle "failed" and "undelivered" callbacks from Twilio + # Handle "MessageStatus" key from Twilio if request.POST.get("MessageStatus") in ("failed", "undelivered"): status = request.POST["MessageStatus"] error = f"Delivery failed (status={status})." + # Handle "CallStatus" key from Twilio + if request.POST.get("CallStatus") == "failed": + error = f"Delivery failed (status=failed)." + if error: notification.error = error notification.save(update_fields=["error"])