Phone provider refactoring

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Changed
+
+- Phone provider refactoring
+
 ### Fixed
 
 - Improve plugin authentication by @vadimkerr ([#1995](https://github.com/grafana/oncall/pull/1995))

engine/apps/alerts/constants.py

@@ -2,7 +2,7 @@ class ActionSource:
     (
         SLACK,
         WEB,
-        TWILIO,
+        PHONE,
         TELEGRAM,
     ) = range(4)
 

engine/apps/alerts/incident_appearance/templaters/phone_call_templater.py

@@ -1,5 +1,5 @@
 from apps.alerts.incident_appearance.templaters.alert_templater import AlertTemplater
-from common.utils import clean_markup, escape_for_twilio_phone_call
+from common.utils import clean_markup
 
 
 class AlertPhoneCallTemplater(AlertTemplater):
@@ -14,14 +14,11 @@ def _postformat(self, templated_alert):
         return templated_alert
 
     def _postformat_pipeline(self, text):
-        return self._escape(clean_markup(self._slack_format_for_phone_call(text))) if text is not None else text
+        return clean_markup(self._slack_format_for_phone_call(text)).replace('"', "") if text is not None else text
 
     def _slack_format_for_phone_call(self, data):
         sf = self.slack_formatter
         sf.user_mention_format = "{}"
         sf.channel_mention_format = "#{}"
         sf.hyperlink_mention_format = "{title}"
         return sf.format(data)
-
-    def _escape(self, data):
-        return escape_for_twilio_phone_call(data)

engine/apps/alerts/tasks/notify_user.py

@@ -9,7 +9,7 @@
 from apps.alerts.constants import NEXT_ESCALATION_DELAY
 from apps.alerts.signals import user_notification_action_triggered_signal
 from apps.base.messaging import get_messaging_backend_from_id
-from apps.base.utils import live_settings
+from apps.phone_notifications.phone_backend import PhoneBackend
 from common.custom_celery_tasks import shared_dedicated_queue_retry_task
 
 from .task_logger import task_logger
@@ -224,8 +224,6 @@ def notify_user_task(
     autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None
 )
 def perform_notification(log_record_pk):
-    SMSMessage = apps.get_model("twilioapp", "SMSMessage")
-    PhoneCall = apps.get_model("twilioapp", "PhoneCall")
     UserNotificationPolicy = apps.get_model("base", "UserNotificationPolicy")
     TelegramToUserConnector = apps.get_model("telegram", "TelegramToUserConnector")
     UserNotificationPolicyLogRecord = apps.get_model("base", "UserNotificationPolicyLogRecord")
@@ -259,20 +257,12 @@ def perform_notification(log_record_pk):
         return
 
     if notification_channel == UserNotificationPolicy.NotificationChannel.SMS:
-        SMSMessage.send_sms(
-            user,
-            alert_group,
-            notification_policy,
-            is_cloud_notification=live_settings.GRAFANA_CLOUD_NOTIFICATIONS_ENABLED,
-        )
+        phone_backend = PhoneBackend()
+        phone_backend.notify_by_sms(user, alert_group, notification_policy)
 
     elif notification_channel == UserNotificationPolicy.NotificationChannel.PHONE_CALL:
-        PhoneCall.make_call(
-            user,
-            alert_group,
-            notification_policy,
-            is_cloud_notification=live_settings.GRAFANA_CLOUD_NOTIFICATIONS_ENABLED,
-        )
+        phone_backend = PhoneBackend()
+        phone_backend.notify_by_call(user, alert_group, notification_policy)
 
     elif notification_channel == UserNotificationPolicy.NotificationChannel.TELEGRAM:
         TelegramToUserConnector.notify_user(user, alert_group, notification_policy)

engine/apps/alerts/tests/test_alert_group.py

@@ -38,7 +38,7 @@ def test_render_for_phone_call(
     )
 
     expected_verbose_name = (
-        f"You are invited to check an incident from Grafana OnCall. "
+        f"to check an incident from Grafana OnCall. "
         f"Alert via {alert_receive_channel.verbal_name} - Grafana with title TestAlert triggered 1 times"
     )
     rendered_text = AlertGroupPhoneCallRenderer(alert_group).render()

engine/apps/api/serializers/organization.py

@@ -1,3 +1,4 @@
+from dataclasses import asdict
 from datetime import timedelta
 
 import humanize
@@ -7,6 +8,7 @@
 from rest_framework import fields, serializers
 
 from apps.base.models import LiveSetting
+from apps.phone_notifications.phone_provider import get_phone_provider
 from apps.slack.models import SlackTeamIdentity
 from apps.slack.tasks import resolve_archived_incidents_for_organization, unarchive_incidents_for_organization
 from apps.user_management.models import Organization
@@ -112,14 +114,16 @@ def get_limits(self, obj):
         return obj.notifications_limit_web_report(user)
 
     def get_env_status(self, obj):
+        # deprecated in favour of ConfigAPIView.
+        # All new env statuses should be added there
         LiveSetting.populate_settings_if_needed()
 
         telegram_configured = not LiveSetting.objects.filter(name__startswith="TELEGRAM", error__isnull=False).exists()
-        twilio_configured = not LiveSetting.objects.filter(name__startswith="TWILIO", error__isnull=False).exists()
-
+        phone_provider_config = get_phone_provider().flags
         return {
             "telegram_configured": telegram_configured,
-            "twilio_configured": twilio_configured,
+            "twilio_configured": phone_provider_config.configured,  # keep for backward compatibility
+            "phone_provider": asdict(phone_provider_config),
         }
 
     def get_stats(self, obj):

engine/apps/api/serializers/user.py

@@ -11,11 +11,11 @@
 from apps.base.models import UserNotificationPolicy
 from apps.base.utils import live_settings
 from apps.oss_installation.utils import cloud_user_identity_status
-from apps.twilioapp.utils import check_phone_number_is_valid
 from apps.user_management.models import User
 from apps.user_management.models.user import default_working_hours
 from common.api_helpers.custom_fields import TeamPrimaryKeyRelatedField
 from common.api_helpers.mixins import EagerLoadingMixin
+from common.api_helpers.utils import check_phone_number_is_valid
 from common.timezones import TimeZoneField
 
 from .custom_serializers import DynamicFieldsModelSerializer

engine/apps/api/tests/test_user.py

@@ -17,6 +17,7 @@
     RBACPermission,
 )
 from apps.base.models import UserNotificationPolicy
+from apps.phone_notifications.exceptions import FailedToFinishVerification
 from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb
 from apps.user_management.models.user import default_working_hours
 
@@ -471,7 +472,7 @@ def test_user_get_other_verification_code(
 
     client = APIClient()
     url = reverse("api-internal:user-get-verification-code", kwargs={"pk": admin.public_primary_key})
-    with patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock()):
+    with patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock()):
         response = client.get(url, format="json", **make_user_auth_headers(tester, token))
 
     assert response.status_code == expected_status
@@ -486,7 +487,7 @@ def test_validation_of_verification_code(
     client = APIClient()
     url = reverse("api-internal:user-verify-number", kwargs={"pk": user.public_primary_key})
     with patch(
-        "apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None)
+        "apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True
     ) as verify_phone_number:
         url_with_token = f"{url}?token=some_token"
         r = client.put(url_with_token, format="json", **make_user_auth_headers(user, token))
@@ -504,6 +505,24 @@ def test_validation_of_verification_code(
         assert verify_phone_number.call_count == 1
 
 
+@pytest.mark.django_db
+def test_verification_code_provider_exception(
+    make_organization_and_user_with_plugin_token,
+    make_user_auth_headers,
+):
+    organization, user, token = make_organization_and_user_with_plugin_token()
+    client = APIClient()
+    url = reverse("api-internal:user-verify-number", kwargs={"pk": user.public_primary_key})
+    with patch(
+        "apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number",
+        side_effect=FailedToFinishVerification,
+    ) as verify_phone_number:
+        url_with_token = f"{url}?token=some_token"
+        r = client.put(url_with_token, format="json", **make_user_auth_headers(user, token))
+        assert r.status_code == 503
+        assert verify_phone_number.call_count == 1
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "role,expected_status",
@@ -561,7 +580,7 @@ def test_user_verify_another_phone(
     client = APIClient()
     url = reverse("api-internal:user-verify-number", kwargs={"pk": other_user.public_primary_key})
 
-    with patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None)):
+    with patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True):
         response = client.put(f"{url}?token=12345", format="json", **make_user_auth_headers(tester, token))
 
     assert response.status_code == expected_status
@@ -686,7 +705,7 @@ def test_admin_can_detail_users(
     assert response.status_code == status.HTTP_200_OK
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
 @pytest.mark.django_db
 def test_admin_can_get_own_verification_code(
     mock_verification_start,
@@ -702,7 +721,7 @@ def test_admin_can_get_own_verification_code(
     assert response.status_code == status.HTTP_200_OK
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
 @pytest.mark.django_db
 def test_admin_can_get_another_user_verification_code(
     mock_verification_start,
@@ -719,7 +738,7 @@ def test_admin_can_get_another_user_verification_code(
     assert response.status_code == status.HTTP_200_OK
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @pytest.mark.django_db
 def test_admin_can_verify_own_phone(
     mocked_verification_check,
@@ -734,7 +753,7 @@ def test_admin_can_verify_own_phone(
     assert response.status_code == status.HTTP_200_OK
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @pytest.mark.django_db
 def test_admin_can_verify_another_user_phone(
     mocked_verification_check,
@@ -912,7 +931,7 @@ def test_user_can_detail_users(
     assert response.status_code == status.HTTP_403_FORBIDDEN
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
 @pytest.mark.django_db
 def test_user_can_get_own_verification_code(
     mock_verification_start, make_organization_and_user_with_plugin_token, make_user_auth_headers
@@ -926,7 +945,7 @@ def test_user_can_get_own_verification_code(
     assert response.status_code == status.HTTP_200_OK
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
 @pytest.mark.django_db
 def test_user_cant_get_another_user_verification_code(
     mock_verification_start,
@@ -944,7 +963,7 @@ def test_user_cant_get_another_user_verification_code(
     assert response.status_code == status.HTTP_403_FORBIDDEN
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @pytest.mark.django_db
 def test_user_can_verify_own_phone(
     mocked_verification_check, make_organization_and_user_with_plugin_token, make_user_auth_headers
@@ -958,7 +977,7 @@ def test_user_can_verify_own_phone(
     assert response.status_code == status.HTTP_200_OK
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @pytest.mark.django_db
 def test_user_cant_verify_another_user_phone(
     mocked_verification_check,
@@ -1218,7 +1237,7 @@ def test_viewer_cant_detail_users(
     assert response.status_code == status.HTTP_403_FORBIDDEN
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
 @pytest.mark.django_db
 def test_viewer_cant_get_own_verification_code(
     mock_verification_start, make_organization_and_user_with_plugin_token, make_user_auth_headers
@@ -1232,7 +1251,7 @@ def test_viewer_cant_get_own_verification_code(
     assert response.status_code == status.HTTP_403_FORBIDDEN
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
 @pytest.mark.django_db
 def test_viewer_cant_get_another_user_verification_code(
     mock_verification_start,
@@ -1250,7 +1269,7 @@ def test_viewer_cant_get_another_user_verification_code(
     assert response.status_code == status.HTTP_403_FORBIDDEN
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @pytest.mark.django_db
 def test_viewer_cant_verify_own_phone(
     mocked_verification_check, make_organization_and_user_with_plugin_token, make_user_auth_headers
@@ -1264,7 +1283,7 @@ def test_viewer_cant_verify_own_phone(
     assert response.status_code == status.HTTP_403_FORBIDDEN
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @pytest.mark.django_db
 def test_viewer_cant_verify_another_user_phone(
     mocked_verification_check,
@@ -1340,9 +1359,7 @@ def test_forget_own_number(
 
     client = APIClient()
     url = reverse("api-internal:user-forget-number", kwargs={"pk": user.public_primary_key})
-    with patch(
-        "apps.twilioapp.phone_manager.PhoneManager.notify_about_changed_verified_phone_number", return_value=None
-    ):
+    with patch("apps.phone_notifications.phone_backend.PhoneBackend._notify_disconnected_number", return_value=None):
         response = client.put(url, None, format="json", **make_user_auth_headers(user, token))
         assert response.status_code == expected_status
 
@@ -1390,9 +1407,7 @@ def test_forget_other_number(
 
     client = APIClient()
     url = reverse("api-internal:user-forget-number", kwargs={"pk": admin_primary_key})
-    with patch(
-        "apps.twilioapp.phone_manager.PhoneManager.notify_about_changed_verified_phone_number", return_value=None
-    ):
+    with patch("apps.phone_notifications.phone_backend.PhoneBackend._notify_disconnected_number", return_value=None):
         response = client.put(url, None, format="json", **make_user_auth_headers(other_user, token))
         assert response.status_code == expected_status
 
@@ -1574,8 +1589,8 @@ def test_check_availability_other_user(make_organization_and_user_with_plugin_to
     assert response.status_code == status.HTTP_200_OK
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @patch(
     "apps.api.throttlers.GetPhoneVerificationCodeThrottlerPerUser.get_throttle_limits",
     return_value=(1, 10 * 60),
@@ -1616,8 +1631,8 @@ def test_phone_number_verification_flow_ratelimit_per_user(
     assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=Mock())
-@patch("apps.twilioapp.phone_manager.PhoneManager.verify_phone_number", return_value=(True, None))
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.verify_phone_number", return_value=True)
 @patch(
     "apps.api.throttlers.GetPhoneVerificationCodeThrottlerPerOrg.get_throttle_limits",
     return_value=(1, 10 * 60),
@@ -1659,7 +1674,7 @@ def test_phone_number_verification_flow_ratelimit_per_org(
     assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS
 
 
-@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=True)
+@patch("apps.phone_notifications.phone_backend.PhoneBackend.send_verification_sms", return_value=Mock())
 @pytest.mark.parametrize(
     "recaptcha_testing_pass,expected_status",
     [
@@ -1686,7 +1701,7 @@ def test_phone_number_verification_recaptcha(
         response = client.get(url, format="json", **request_headers)
         assert response.status_code == expected_status
         if expected_status == status.HTTP_200_OK:
-            mock_verification_start.assert_called_once_with()
+            mock_verification_start.assert_called_once_with(user)
         else:
             mock_verification_start.assert_not_called()