Listen for alert group actions during whole notification call (#5282)

1. Wrap whole message in twiml <Gather> - that's an actual fix 2. Use twilio helper lib to build twiml queries 3. URLencode twimlquery only once before making a call to reduce code duplication. --------- Co-authored-by: Joey Orlando <[email protected]>
grafana · Nov 22, 2024 · 03ff4c5 · 03ff4c5
1 parent 1b4cca9
commit 03ff4c5
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 28 deletions.
diff --git a/engine/apps/twilioapp/gather.py b/engine/apps/twilioapp/gather.py
@@ -24,16 +24,22 @@ def process_gather_data(call_sid: str, digit: str) -> VoiceResponse:
 
     response = VoiceResponse()
 
+    success_messages = {
+        "1": "Acknowledged",
+        "2": "Resolved",
+        "3": "Silenced",
+    }
     if digit in ["1", "2", "3"]:
         # Success case
-        response.say(f"You have pressed digit {digit}")
+        msg = success_messages.get(digit, f"You have pressed digit {digit}")
+        response.say(msg)
         process_digit(call_sid, digit)
     else:
         # Error wrong digit pressing
         gather = Gather(action=get_gather_url(), method="POST", num_digits=1)
 
         response.say("Wrong digit")
-        gather.say(get_gather_message())
+        gather.say(get_alert_group_gather_instructions())
 
         response.append(gather)
 
@@ -85,5 +91,5 @@ def get_gather_url():
     return create_engine_url(reverse("twilioapp:gather"))
 
 
-def get_gather_message():
-    return "Press 1 to acknowledge, 2 to resolve, 3 to silence to 30 minutes"
+def get_alert_group_gather_instructions():
+    return "Press 1 to acknowledge, 2 to resolve, 3 to silence for 30 minutes"
diff --git a/engine/apps/twilioapp/phone_provider.py b/engine/apps/twilioapp/phone_provider.py
@@ -6,6 +6,7 @@
 from phonenumbers import COUNTRY_CODE_TO_REGION_CODE
 from twilio.base.exceptions import TwilioRestException
 from twilio.rest import Client
+from twilio.twiml.voice_response import Gather, Say, VoiceResponse
 
 from apps.base.models import LiveSetting
 from apps.base.utils import live_settings
@@ -16,7 +17,7 @@
     FailedToStartVerification,
 )
 from apps.phone_notifications.phone_provider import PhoneProvider, ProviderFlags
-from apps.twilioapp.gather import get_gather_message, get_gather_url
+from apps.twilioapp.gather import get_alert_group_gather_instructions, get_gather_url
 from apps.twilioapp.models import (
     TwilioCallStatuses,
     TwilioPhoneCall,
@@ -34,13 +35,13 @@ class TwilioPhoneProvider(PhoneProvider):
     def make_notification_call(self, number: str, message: str) -> TwilioPhoneCall | None:
         message = self._escape_call_message(message)
 
-        twiml_query = self._message_to_twiml(message, with_gather=True)
+        twiml = self._message_to_twiml_gather(message)
 
         response = None
         try_without_callback = False
 
         try:
-            response = self._call_create(twiml_query, number, with_callback=True)
+            response = self._call_create(twiml, number, with_callback=True)
         except TwilioRestException as e:
             # If status callback is not valid and not accessible from public url then trying to send message without it
             # https://www.twilio.com/docs/api/errors/21609
@@ -53,7 +54,7 @@ def make_notification_call(self, number: str, message: str) -> TwilioPhoneCall |
 
         if try_without_callback:
             try:
-                response = self._call_create(twiml_query, number, with_callback=False)
+                response = self._call_create(twiml, number, with_callback=False)
             except TwilioRestException as e:
                 logger.error(f"TwilioPhoneProvider.make_notification_call: failed {e}")
                 raise FailedToMakeCall(graceful_msg=self._get_graceful_msg(e, number))
@@ -146,9 +147,9 @@ def _get_graceful_msg(self, e, number):
         return None
 
     def make_call(self, number: str, message: str):
-        twiml_query = self._message_to_twiml(message, with_gather=False)
+        twiml = self._message_to_twiml_say(message)
         try:
-            self._call_create(twiml_query, number, with_callback=False)
+            self._call_create(twiml, number, with_callback=False)
         except TwilioRestException as e:
             logger.error(f"TwilioPhoneProvider.make_call: failed {e}")
             raise FailedToMakeCall(graceful_msg=self._get_graceful_msg(e, number))
@@ -160,18 +161,25 @@ def send_sms(self, number: str, message: str):
             logger.error(f"TwilioPhoneProvider.send_sms: failed {e}")
             raise FailedToSendSMS(graceful_msg=self._get_graceful_msg(e, number))
 
-    def _message_to_twiml(self, message: str, with_gather=False):
-        q = f"<Response><Say>{message}</Say></Response>"
-        if with_gather:
-            gather_subquery = f'<Gather numDigits="1" action="{get_gather_url()}" method="POST"><Say>{get_gather_message()}</Say></Gather>'
-            q = f"<Response><Say>{message}</Say>{gather_subquery}</Response>"
-        return urllib.parse.quote(
-            q,
-            safe="",
-        )
-
-    def _call_create(self, twiml_query: str, to: str, with_callback: bool):
+    def _message_to_twiml_say(self, message: str) -> VoiceResponse:
+        response = VoiceResponse()
+        say = Say(message)
+        response.append(say)
+        return response
+
+    def _message_to_twiml_gather(self, message: str) -> VoiceResponse:
+        response = VoiceResponse()
+        gather = Gather(action=get_gather_url(), method="POST", num_digits=1)
+        gather.say(message)
+        gather.pause(length=1)
+        gather.say(get_alert_group_gather_instructions())
+        response.append(gather)
+        return response
+
+    def _call_create(self, twiml: VoiceResponse, to: str, with_callback: bool):
         client, from_ = self._phone_sender(to)
+        # encode twiml VoiceResponse to use in url
+        twiml_query = urllib.parse.quote(str(twiml), safe="")
         url = "http://twimlets.com/echo?Twiml=" + twiml_query
         if with_callback:
             status_callback = get_call_status_callback_url()

diff --git a/engine/apps/twilioapp/tests/test_phone_calls.py b/engine/apps/twilioapp/tests/test_phone_calls.py
@@ -138,7 +138,7 @@ def test_acknowledge_by_phone(mock_has_permission, mock_get_gather_url, make_twi
     content = response.content.decode("utf-8")
 
     assert response.status_code == 200
-    assert "You have pressed digit 1" in content
+    assert "Acknowledged" in content
 
     alert_group.refresh_from_db()
     assert alert_group.acknowledged is True
@@ -173,7 +173,7 @@ def test_resolve_by_phone(mock_has_permission, mock_get_gather_url, make_twilio_
     content = BeautifulSoup(content, features="xml").findAll(string=True)
 
     assert response.status_code == 200
-    assert "You have pressed digit 2" in content
+    assert "Resolved" in content
 
     alert_group.refresh_from_db()
     assert alert_group.resolved is True
@@ -207,7 +207,7 @@ def test_silence_by_phone(mock_has_permission, mock_get_gather_url, make_twilio_
     content = response.content.decode("utf-8")
 
     assert response.status_code == 200
-    assert "You have pressed digit 3" in content
+    assert "Silenced" in content
 
     alert_group.refresh_from_db()
     assert alert_group.silenced_until is not None

diff --git a/engine/apps/twilioapp/tests/test_twilio_provider.py b/engine/apps/twilioapp/tests/test_twilio_provider.py
@@ -19,28 +19,32 @@ class MockTwilioMessageInstance:
 
 @pytest.mark.django_db
 @mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._call_create", return_value=MockTwilioCallInstance())
-@mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml", return_value="mocked_twiml")
+@mock.patch(
+    "apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml_gather", return_value="twiml_gather_response"
+)
 def test_make_notification_call(mock_twiml, mock_call_create):
     number = "+1234567890"
     message = "Hello"
     provider = TwilioPhoneProvider()
     provider_call = provider.make_notification_call(number, message)
-    mock_call_create.assert_called_once_with("mocked_twiml", number, with_callback=True)
+    mock_call_create.assert_called_once_with("twiml_gather_response", number, with_callback=True)
     assert provider_call is not None
     assert provider_call.sid == MockTwilioCallInstance.sid
     assert provider_call.id is None  # test that provider_call is returned by notification call and NOT saved
 
 
 @pytest.mark.django_db
 @mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._call_create", return_value=MockTwilioCallInstance())
-@mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml", return_value="mocked_twiml")
+@mock.patch(
+    "apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml_say", return_value="twiml_say_response"
+)
 def test_make_call(mock_twiml, mock_call_create):
     number = "+1234567890"
     message = "Hello"
     provider = TwilioPhoneProvider()
     provider_call = provider.make_call(number, message)
     assert provider_call is None  # test that provider_call is not returned from make_call
-    mock_call_create.assert_called_once_with("mocked_twiml", number, with_callback=False)
+    mock_call_create.assert_called_once_with("twiml_say_response", number, with_callback=False)
 
 
 class MockTwilioSMSInstance: