From 0db955502c5a6791dbdeba318bb176761eed006d Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 18 May 2023 20:33:37 -0600 Subject: [PATCH 1/9] WIP map phone numbers to different twilio resources --- ...wiliosmssender_twilioverificationsender.py | 77 +++++++++++++++++++ engine/apps/twilioapp/models/twilio_sender.py | 37 +++++++++ engine/apps/twilioapp/twilio_client.py | 61 +++++++++++---- 3 files changed, 158 insertions(+), 17 deletions(-) create mode 100644 engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py create mode 100644 engine/apps/twilioapp/models/twilio_sender.py diff --git a/engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py b/engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py new file mode 100644 index 0000000000..94200cfab0 --- /dev/null +++ b/engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py @@ -0,0 +1,77 @@ +# Generated by Django 3.2.18 on 2023-05-19 01:58 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('twilioapp', '0002_auto_20220604_1008'), + ] + + operations = [ + migrations.CreateModel( + name='TwilioAccount', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=100)), + ('account_sid', models.CharField(max_length=64, unique=True)), + ('auth_token', models.CharField(default=None, max_length=64, null=True)), + ('api_key_sid', models.CharField(default=None, max_length=64, null=True)), + ('api_key_secret', models.CharField(default=None, max_length=64, null=True)), + ], + ), + migrations.CreateModel( + name='TwilioSender', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(default='Default', max_length=100)), + ('country_code_name', models.CharField(default='Default', max_length=100)), + ('country_code', models.CharField(default=None, max_length=16, null=True)), + ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='twilio_sender', to='twilioapp.twilioaccount')), + ('polymorphic_ctype', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='polymorphic_twilioapp.twiliosender_set+', to='contenttypes.contenttype')), + ], + options={ + 'abstract': False, + 'base_manager_name': 'objects', + }, + ), + migrations.CreateModel( + name='TwilioPhoneCallSender', + fields=[ + ('twiliosender_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='twilioapp.twiliosender')), + ('number', models.CharField(max_length=16)), + ], + options={ + 'abstract': False, + 'base_manager_name': 'objects', + }, + bases=('twilioapp.twiliosender',), + ), + migrations.CreateModel( + name='TwilioSmsSender', + fields=[ + ('twiliosender_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='twilioapp.twiliosender')), + ('sender', models.CharField(max_length=16)), + ], + options={ + 'abstract': False, + 'base_manager_name': 'objects', + }, + bases=('twilioapp.twiliosender',), + ), + migrations.CreateModel( + name='TwilioVerificationSender', + fields=[ + ('twiliosender_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='twilioapp.twiliosender')), + ('verify_service_sid', models.CharField(max_length=64)), + ], + options={ + 'abstract': False, + 'base_manager_name': 'objects', + }, + bases=('twilioapp.twiliosender',), + ), + ] diff --git a/engine/apps/twilioapp/models/twilio_sender.py b/engine/apps/twilioapp/models/twilio_sender.py new file mode 100644 index 0000000000..861655ac2a --- /dev/null +++ b/engine/apps/twilioapp/models/twilio_sender.py @@ -0,0 +1,37 @@ +from django.db import models +from polymorphic.models import PolymorphicModel +from twilio.rest import Client + + +class TwilioAccount(models.Model): + name = models.CharField(max_length=100) + account_sid = models.CharField(max_length=64, null=False, blank=False, unique=True) + auth_token = models.CharField(max_length=64, null=True, default=None) + api_key_sid = models.CharField(max_length=64, null=True, default=None) + api_key_secret = models.CharField(max_length=64, null=True, default=None) + + def get_twilio_api_client(self): + if self.api_key_sid and self.api_key_secret: + return Client(self.api_key_sid, self.api_key_secret, self.account_sid) + else: + return Client(self.account_sid, self.auth_token) + + +class TwilioSender(PolymorphicModel): + name = models.CharField(max_length=100, null=False, default="Default") + country_code_name = models.CharField(max_length=100, null=False, default="Default") + country_code = models.CharField(max_length=16, null=True, default=None) + account = models.ForeignKey("twilioapp.TwilioAccount", on_delete=models.CASCADE, related_name="twilio_sender") + + +class TwilioSmsSender(TwilioSender): + # Sender for sms is phone number, short code or alphanumeric id + sender = models.CharField(max_length=16, null=False, blank=False) + + +class TwilioPhoneCallSender(TwilioSender): + number = models.CharField(max_length=16, null=False, blank=False) + + +class TwilioVerificationSender(TwilioSender): + verify_service_sid = models.CharField(max_length=64, null=False, blank=False) diff --git a/engine/apps/twilioapp/twilio_client.py b/engine/apps/twilioapp/twilio_client.py index 75d0640382..fb512ed8d0 100644 --- a/engine/apps/twilioapp/twilio_client.py +++ b/engine/apps/twilioapp/twilio_client.py @@ -2,12 +2,14 @@ import urllib.parse from django.apps import apps +from django.db.models import Q from django.urls import reverse from twilio.base.exceptions import TwilioRestException from twilio.rest import Client from apps.base.utils import live_settings from apps.twilioapp.constants import TEST_CALL_TEXT, TwilioLogRecordStatus, TwilioLogRecordType +from apps.twilioapp.models.twilio_sender import TwilioPhoneCallSender, TwilioSmsSender, TwilioVerificationSender from apps.twilioapp.utils import get_calling_code, get_gather_message, get_gather_url, parse_phone_number from common.api_helpers.utils import create_engine_url @@ -16,7 +18,7 @@ class TwilioClient: @property - def twilio_api_client(self): + def default_twilio_api_client(self): if live_settings.TWILIO_API_KEY_SID and live_settings.TWILIO_API_KEY_SECRET: return Client( live_settings.TWILIO_API_KEY_SID, live_settings.TWILIO_API_KEY_SECRET, live_settings.TWILIO_ACCOUNT_SID @@ -25,27 +27,51 @@ def twilio_api_client(self): return Client(live_settings.TWILIO_ACCOUNT_SID, live_settings.TWILIO_AUTH_TOKEN) @property - def twilio_number(self): + def default_twilio_number(self): return live_settings.TWILIO_NUMBER + def twilio_sender(self, sender_type, to, accessor): + _, _, country_code = self.parse_number(to) + TwilioSender = apps.get_model("twilioapp", "TwilioSender") + sender = ( + TwilioSender.objects.instance_of(sender_type) + .filter(Q(country_code=country_code) | Q(country_code__isnull=True)) + .order_by("-country_code") + .first() + ) + + client = self.default_twilio_api_client + if sender: + client = sender.account.get_twilio_api_client() + + return client, accessor(sender) + + def sms_accessor(self, sender): + return sender.sender if sender else self.default_twilio_number + + def phone_accessor(self, sender): + return sender.number if sender else self.default_twilio_number + + def verify_accessor(self, sender): + return sender.verify_service_sid if sender else live_settings.TWILIO_VERIFY_SERVICE_SID + def send_message(self, body, to): + client, from_ = self.twilio_sender(TwilioSmsSender, to, self.sms_accessor) status_callback = create_engine_url(reverse("twilioapp:sms_status_events")) try: - return self.twilio_api_client.messages.create( - body=body, to=to, from_=self.twilio_number, status_callback=status_callback - ) + return client.messages.create(body=body, to=to, from_=from_, status_callback=status_callback) 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 if e.code == 21609: logger.warning("twilio_client.send_message: Twilio error 21609. Status Callback is not public url") - return self.twilio_api_client.messages.create(body=body, to=to, from_=self.twilio_number) + return client.messages.create(body=body, to=to, from_=from_) raise e # Use responsibly def parse_number(self, number): try: - response = self.twilio_api_client.lookups.phone_numbers(number).fetch() + response = self.default_twilio_api_client.lookups.phone_numbers(number).fetch() return True, response.phone_number, get_calling_code(response.country_code) except TwilioRestException as e: if e.code == 20404: @@ -59,11 +85,10 @@ def parse_number(self, number): def verification_start_via_twilio(self, user, phone_number, via): # https://www.twilio.com/docs/verify/api/verification?code-sample=code-start-a-verification-with-sms&code-language=Python&code-sdk-version=6.x + client, verify_service_sid = self.twilio_sender(TwilioVerificationSender, phone_number, self.verify_accessor) verification = None try: - verification = self.twilio_api_client.verify.services( - live_settings.TWILIO_VERIFY_SERVICE_SID - ).verifications.create(to=phone_number, channel=via) + verification = client.verify.services(verify_service_sid).verifications.create(to=phone_number, channel=via) except TwilioRestException as e: logger.error(f"Twilio verification start error: {e} for User: {user.pk}") @@ -92,11 +117,12 @@ def verification_start_via_twilio(self, user, phone_number, via): def verification_check_via_twilio(self, user, phone_number, code): # https://www.twilio.com/docs/verify/api/verification-check?code-sample=code-check-a-verification-with-a-phone-number&code-language=Python&code-sdk-version=6.x + client, verify_service_sid = self.twilio_sender(TwilioVerificationSender, phone_number, self.verify_accessor) succeed = False try: - verification_check = self.twilio_api_client.verify.services( - live_settings.TWILIO_VERIFY_SERVICE_SID - ).verification_checks.create(to=phone_number, code=code) + verification_check = client.verify.services(verify_service_sid).verification_checks.create( + to=phone_number, code=code + ) except TwilioRestException as e: logger.error(f"Twilio verification check error: {e} for User: {user.pk}") self.create_log_record( @@ -132,6 +158,7 @@ def make_test_call(self, to): self.make_call(message=message, to=to) def make_call(self, message, to, grafana_cloud=False): + client, number = self.twilio_sender(TwilioPhoneCallSender, to, self.phone_accessor) try: start_message = message.replace('"', "") @@ -155,10 +182,10 @@ def make_call(self, message, to, grafana_cloud=False): status_callback_events = ["initiated", "ringing", "answered", "completed"] - return self.twilio_api_client.calls.create( + return client.calls.create( url=url, to=to, - from_=self.twilio_number, + from_=number, method="GET", status_callback=status_callback, status_callback_event=status_callback_events, @@ -169,10 +196,10 @@ def make_call(self, message, to, grafana_cloud=False): # https://www.twilio.com/docs/api/errors/21609 if e.code == 21609: logger.warning("twilio_client.make_call: Twilio error 21609. Status Callback is not public url") - return self.twilio_api_client.calls.create( + return client.calls.create( url=url, to=to, - from_=self.twilio_number, + from_=number, method="GET", ) From a9c3817d546d794f7adc38b807049f28bedea84a Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 23 May 2023 09:36:22 -0600 Subject: [PATCH 2/9] Fix live settings test --- engine/apps/base/tests/test_live_settings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/engine/apps/base/tests/test_live_settings.py b/engine/apps/base/tests/test_live_settings.py index 498be849de..972b0b7540 100644 --- a/engine/apps/base/tests/test_live_settings.py +++ b/engine/apps/base/tests/test_live_settings.py @@ -67,6 +67,6 @@ def test_twilio_respects_changed_credentials(settings): live_settings.TWILIO_AUTH_TOKEN = "new_twilio_auth_token" live_settings.TWILIO_NUMBER = "new_twilio_number" - assert twilio_client.twilio_api_client.username == "new_twilio_account_sid" - assert twilio_client.twilio_api_client.password == "new_twilio_auth_token" - assert twilio_client.twilio_number == "new_twilio_number" + assert twilio_client.default_twilio_api_client.username == "new_twilio_account_sid" + assert twilio_client.default_twilio_api_client.password == "new_twilio_auth_token" + assert twilio_client.default_twilio_number == "new_twilio_number" From 352bf51f4aa46f85a863ad288cbf2dc2b344ecef Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 23 May 2023 15:04:10 -0600 Subject: [PATCH 3/9] Add tests, lint --- ...wiliosmssender_twilioverificationsender.py | 3 +- engine/apps/twilioapp/models/twilio_sender.py | 2 +- engine/apps/twilioapp/tests/conftest.py | 40 +++++ engine/apps/twilioapp/tests/factories.py | 26 ++++ engine/apps/twilioapp/tests/test_senders.py | 147 ++++++++++++++++++ engine/apps/twilioapp/twilio_client.py | 33 ++-- 6 files changed, 231 insertions(+), 20 deletions(-) create mode 100644 engine/apps/twilioapp/tests/conftest.py create mode 100644 engine/apps/twilioapp/tests/test_senders.py diff --git a/engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py b/engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py index 94200cfab0..274eeba7d0 100644 --- a/engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py +++ b/engine/apps/twilioapp/migrations/0003_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.18 on 2023-05-19 01:58 +# Generated by Django 3.2.18 on 2023-05-23 18:59 from django.db import migrations, models import django.db.models.deletion @@ -28,7 +28,6 @@ class Migration(migrations.Migration): fields=[ ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('name', models.CharField(default='Default', max_length=100)), - ('country_code_name', models.CharField(default='Default', max_length=100)), ('country_code', models.CharField(default=None, max_length=16, null=True)), ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='twilio_sender', to='twilioapp.twilioaccount')), ('polymorphic_ctype', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='polymorphic_twilioapp.twiliosender_set+', to='contenttypes.contenttype')), diff --git a/engine/apps/twilioapp/models/twilio_sender.py b/engine/apps/twilioapp/models/twilio_sender.py index 861655ac2a..a951895a7a 100644 --- a/engine/apps/twilioapp/models/twilio_sender.py +++ b/engine/apps/twilioapp/models/twilio_sender.py @@ -19,7 +19,7 @@ def get_twilio_api_client(self): class TwilioSender(PolymorphicModel): name = models.CharField(max_length=100, null=False, default="Default") - country_code_name = models.CharField(max_length=100, null=False, default="Default") + # Note: country_code does not have + prefix here country_code = models.CharField(max_length=16, null=True, default=None) account = models.ForeignKey("twilioapp.TwilioAccount", on_delete=models.CASCADE, related_name="twilio_sender") diff --git a/engine/apps/twilioapp/tests/conftest.py b/engine/apps/twilioapp/tests/conftest.py new file mode 100644 index 0000000000..f2064089b9 --- /dev/null +++ b/engine/apps/twilioapp/tests/conftest.py @@ -0,0 +1,40 @@ +import pytest + +from apps.twilioapp.tests.factories import ( + TwilioAccountFactory, + TwilioPhoneCallSenderFactory, + TwilioSmsSenderFactory, + TwilioVerificationSenderFactory, +) + + +@pytest.fixture +def make_twilio_account(): + def _make_twilio_account(**kwargs): + return TwilioAccountFactory(**kwargs) + + return _make_twilio_account + + +@pytest.fixture +def make_twilio_phone_call_sender(): + def _make_twilio_phone_call_sender(**kwargs): + return TwilioPhoneCallSenderFactory(**kwargs) + + return _make_twilio_phone_call_sender + + +@pytest.fixture +def make_twilio_sms_sender(): + def _make_twilio_sms_sender(**kwargs): + return TwilioSmsSenderFactory(**kwargs) + + return _make_twilio_sms_sender + + +@pytest.fixture +def make_twilio_verification_sender(): + def _make_twilio_verification_sender(**kwargs): + return TwilioVerificationSenderFactory(**kwargs) + + return _make_twilio_verification_sender diff --git a/engine/apps/twilioapp/tests/factories.py b/engine/apps/twilioapp/tests/factories.py index e1b49940ca..ad5124a033 100644 --- a/engine/apps/twilioapp/tests/factories.py +++ b/engine/apps/twilioapp/tests/factories.py @@ -1,6 +1,12 @@ import factory from apps.twilioapp.models import PhoneCall, SMSMessage +from apps.twilioapp.models.twilio_sender import ( + TwilioAccount, + TwilioPhoneCallSender, + TwilioSmsSender, + TwilioVerificationSender, +) class PhoneCallFactory(factory.DjangoModelFactory): @@ -11,3 +17,23 @@ class Meta: class SMSFactory(factory.DjangoModelFactory): class Meta: model = SMSMessage + + +class TwilioAccountFactory(factory.DjangoModelFactory): + class Meta: + model = TwilioAccount + + +class TwilioPhoneCallSenderFactory(factory.DjangoModelFactory): + class Meta: + model = TwilioPhoneCallSender + + +class TwilioSmsSenderFactory(factory.DjangoModelFactory): + class Meta: + model = TwilioSmsSender + + +class TwilioVerificationSenderFactory(factory.DjangoModelFactory): + class Meta: + model = TwilioVerificationSender diff --git a/engine/apps/twilioapp/tests/test_senders.py b/engine/apps/twilioapp/tests/test_senders.py new file mode 100644 index 0000000000..6bd199e92d --- /dev/null +++ b/engine/apps/twilioapp/tests/test_senders.py @@ -0,0 +1,147 @@ +from unittest.mock import patch + +import pytest +from django.conf import settings + +from apps.twilioapp.twilio_client import twilio_client + +US_VERIFY = "us_verify" +US_SMS = "us_sms" +US_PHONE = "us_phone" + +DB_DEFAULT_VERIFY = "db_default_verify" +DB_DEFAULT_SMS = "db_default_sms" +DB_DEFAULT_PHONE = "db_default_phone" + +DB_TWILIO_AUTH_TOKEN = "db_twilio_account_auth_token" +DB_TWILIO_ACCOUNT_SID = "db_twilio_account_sid" + +ENV_VERIFY_SERVICE_SID = "env_twilio_verify_service_sid" +ENV_TWILIO_NUMBER = "env_twilio_number" +ENV_TWILIO_AUTH_TOKEN = "env_twilio_auth_token" +ENV_TWILIO_ACCOUNT_SID = "env_twilio_account_sid" + + +@pytest.fixture +def setup_env_default_twilio(): + settings.TWILIO_ACCOUNT_SID = ENV_TWILIO_ACCOUNT_SID + settings.TWILIO_AUTH_TOKEN = ENV_TWILIO_AUTH_TOKEN + settings.TWILIO_NUMBER = ENV_TWILIO_NUMBER + settings.TWILIO_VERIFY_SERVICE_SID = ENV_VERIFY_SERVICE_SID + + +@pytest.fixture +def setup_db_default_account(make_twilio_account): + return make_twilio_account( + name="DB Twilio Account", account_sid=DB_TWILIO_ACCOUNT_SID, auth_token=DB_TWILIO_AUTH_TOKEN + ) + + +@pytest.fixture +def setup_default_senders( + setup_db_default_account, make_twilio_phone_call_sender, make_twilio_sms_sender, make_twilio_verification_sender +): + make_twilio_phone_call_sender(name="Default", number=DB_DEFAULT_PHONE, account=setup_db_default_account) + make_twilio_sms_sender(name="Default", sender=DB_DEFAULT_SMS, account=setup_db_default_account) + make_twilio_verification_sender( + name="Default", verify_service_sid=DB_DEFAULT_VERIFY, account=setup_db_default_account + ) + + +@pytest.fixture +def setup_us_senders( + setup_db_default_account, make_twilio_phone_call_sender, make_twilio_sms_sender, make_twilio_verification_sender +): + make_twilio_phone_call_sender(name="US/Canada", country_code="1", number=US_PHONE, account=setup_db_default_account) + make_twilio_sms_sender(name="US/Canada", country_code="1", sender=US_SMS, account=setup_db_default_account) + make_twilio_verification_sender( + name="US/Canada", country_code="1", verify_service_sid=US_VERIFY, account=setup_db_default_account + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "sender,expected_from", + [ + (twilio_client._phone_sender, ENV_TWILIO_NUMBER), + (twilio_client._sms_sender, ENV_TWILIO_NUMBER), + (twilio_client._verify_sender, ENV_VERIFY_SERVICE_SID), + ], +) +def test_use_env_default_senders( + setup_env_default_twilio, + setup_us_senders, + make_twilio_account, + make_twilio_phone_call_sender, + make_twilio_sms_sender, + make_twilio_verification_sender, + sender, + expected_from, +): + with patch( + "apps.twilioapp.twilio_client.TwilioClient.parse_number", + return_value=(True, None, "44"), + ): + client, _from = sender("") + assert _from == expected_from + assert client.username == ENV_TWILIO_ACCOUNT_SID + assert client.password == ENV_TWILIO_AUTH_TOKEN + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "sender,expected_from", + [ + (twilio_client._phone_sender, DB_DEFAULT_PHONE), + (twilio_client._sms_sender, DB_DEFAULT_SMS), + (twilio_client._verify_sender, DB_DEFAULT_VERIFY), + ], +) +def test_use_db_default_senders( + setup_env_default_twilio, + setup_default_senders, + make_twilio_account, + make_twilio_phone_call_sender, + make_twilio_sms_sender, + make_twilio_verification_sender, + sender, + expected_from, +): + with patch( + "apps.twilioapp.twilio_client.TwilioClient.parse_number", + return_value=(True, None, "44"), + ): + client, _from = sender("") + assert _from == expected_from + assert client.username == DB_TWILIO_ACCOUNT_SID + assert client.password == DB_TWILIO_AUTH_TOKEN + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "sender,expected_from", + [ + (twilio_client._phone_sender, US_PHONE), + (twilio_client._sms_sender, US_SMS), + (twilio_client._verify_sender, US_VERIFY), + ], +) +def test_use_country_code_senders( + setup_env_default_twilio, + setup_default_senders, + setup_us_senders, + make_twilio_account, + make_twilio_phone_call_sender, + make_twilio_sms_sender, + make_twilio_verification_sender, + sender, + expected_from, +): + with patch( + "apps.twilioapp.twilio_client.TwilioClient.parse_number", + return_value=(True, None, "1"), + ): + client, _from = sender("") + assert _from == expected_from + assert client.username == DB_TWILIO_ACCOUNT_SID + assert client.password == DB_TWILIO_AUTH_TOKEN diff --git a/engine/apps/twilioapp/twilio_client.py b/engine/apps/twilioapp/twilio_client.py index fb512ed8d0..a274cd9122 100644 --- a/engine/apps/twilioapp/twilio_client.py +++ b/engine/apps/twilioapp/twilio_client.py @@ -30,7 +30,7 @@ def default_twilio_api_client(self): def default_twilio_number(self): return live_settings.TWILIO_NUMBER - def twilio_sender(self, sender_type, to, accessor): + def _twilio_sender(self, sender_type, to): _, _, country_code = self.parse_number(to) TwilioSender = apps.get_model("twilioapp", "TwilioSender") sender = ( @@ -39,24 +39,23 @@ def twilio_sender(self, sender_type, to, accessor): .order_by("-country_code") .first() ) + client = sender.account.get_twilio_api_client() if sender else self.default_twilio_api_client + return client, sender - client = self.default_twilio_api_client - if sender: - client = sender.account.get_twilio_api_client() + def _sms_sender(self, to): + client, sender = self._twilio_sender(TwilioSmsSender, to) + return client, sender.sender if sender else self.default_twilio_number - return client, accessor(sender) + def _phone_sender(self, to): + client, sender = self._twilio_sender(TwilioPhoneCallSender, to) + return client, sender.number if sender else self.default_twilio_number - def sms_accessor(self, sender): - return sender.sender if sender else self.default_twilio_number - - def phone_accessor(self, sender): - return sender.number if sender else self.default_twilio_number - - def verify_accessor(self, sender): - return sender.verify_service_sid if sender else live_settings.TWILIO_VERIFY_SERVICE_SID + def _verify_sender(self, to): + client, sender = self._twilio_sender(TwilioVerificationSender, to) + return client, sender.verify_service_sid if sender else live_settings.TWILIO_VERIFY_SERVICE_SID def send_message(self, body, to): - client, from_ = self.twilio_sender(TwilioSmsSender, to, self.sms_accessor) + client, from_ = self._sms_sender(to) status_callback = create_engine_url(reverse("twilioapp:sms_status_events")) try: return client.messages.create(body=body, to=to, from_=from_, status_callback=status_callback) @@ -85,7 +84,7 @@ def parse_number(self, number): def verification_start_via_twilio(self, user, phone_number, via): # https://www.twilio.com/docs/verify/api/verification?code-sample=code-start-a-verification-with-sms&code-language=Python&code-sdk-version=6.x - client, verify_service_sid = self.twilio_sender(TwilioVerificationSender, phone_number, self.verify_accessor) + client, verify_service_sid = self._verify_sender(phone_number) verification = None try: verification = client.verify.services(verify_service_sid).verifications.create(to=phone_number, channel=via) @@ -117,7 +116,7 @@ def verification_start_via_twilio(self, user, phone_number, via): def verification_check_via_twilio(self, user, phone_number, code): # https://www.twilio.com/docs/verify/api/verification-check?code-sample=code-check-a-verification-with-a-phone-number&code-language=Python&code-sdk-version=6.x - client, verify_service_sid = self.twilio_sender(TwilioVerificationSender, phone_number, self.verify_accessor) + client, verify_service_sid = self._verify_sender(phone_number) succeed = False try: verification_check = client.verify.services(verify_service_sid).verification_checks.create( @@ -158,7 +157,7 @@ def make_test_call(self, to): self.make_call(message=message, to=to) def make_call(self, message, to, grafana_cloud=False): - client, number = self.twilio_sender(TwilioPhoneCallSender, to, self.phone_accessor) + client, number = self._phone_sender(to) try: start_message = message.replace('"', "") From 6f1a4ec8660dc5c34a13ba3acac8db8aa8ba87a0 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 23 May 2023 15:10:30 -0600 Subject: [PATCH 4/9] Update CHANGELOG.md --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78bfcd28c5..cab88ead17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Added + +- Add models and framework to use different services (Phone, SMS, Verify) in Twilio depending on +the destination country code by @mderynck ([#1976](https://github.com/grafana/oncall/pull/1976)) + ## v1.2.27 (2023-05-23) ### Added From 7a451acee820eb9a23cb4208b202469f9a110589 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 23 May 2023 15:53:24 -0600 Subject: [PATCH 5/9] Order for default db sender --- engine/apps/twilioapp/twilio_client.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/engine/apps/twilioapp/twilio_client.py b/engine/apps/twilioapp/twilio_client.py index a274cd9122..32cd4b6a39 100644 --- a/engine/apps/twilioapp/twilio_client.py +++ b/engine/apps/twilioapp/twilio_client.py @@ -33,14 +33,17 @@ def default_twilio_number(self): def _twilio_sender(self, sender_type, to): _, _, country_code = self.parse_number(to) TwilioSender = apps.get_model("twilioapp", "TwilioSender") - sender = ( - TwilioSender.objects.instance_of(sender_type) - .filter(Q(country_code=country_code) | Q(country_code__isnull=True)) - .order_by("-country_code") - .first() + senders = list( + TwilioSender.objects.instance_of(sender_type).filter( + Q(country_code=country_code) | Q(country_code__isnull=True) + ) ) - client = sender.account.get_twilio_api_client() if sender else self.default_twilio_api_client - return client, sender + senders.sort(key=lambda x: (not x.country_code, x)) + + if senders: + return senders[0].account.get_twilio_api_client(), senders[0] + + return self.default_twilio_api_client, None def _sms_sender(self, to): client, sender = self._twilio_sender(TwilioSmsSender, to) From f704e1ccad558b421c9c3b207aab8f752d246f70 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 24 May 2023 14:20:07 -0600 Subject: [PATCH 6/9] Fix test --- engine/apps/base/tests/test_live_settings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/engine/apps/base/tests/test_live_settings.py b/engine/apps/base/tests/test_live_settings.py index 9cdd0e601d..965e2fb2a9 100644 --- a/engine/apps/base/tests/test_live_settings.py +++ b/engine/apps/base/tests/test_live_settings.py @@ -67,6 +67,6 @@ def test_twilio_respects_changed_credentials(settings): live_settings.TWILIO_AUTH_TOKEN = "new_twilio_auth_token" live_settings.TWILIO_NUMBER = "new_twilio_number" - assert twilio_client.default_twilio_api_client.username == "new_twilio_account_sid" - assert twilio_client.default_twilio_api_client.password == "new_twilio_auth_token" - assert twilio_client.default_twilio_number == "new_twilio_number" + assert twilio_client._default_twilio_api_client.username == "new_twilio_account_sid" + assert twilio_client._default_twilio_api_client.password == "new_twilio_auth_token" + assert twilio_client._default_twilio_number == "new_twilio_number" From 48ac7debf2d70a2b31969d5b400f5bf06ead8e5f Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 25 May 2023 10:25:21 -0600 Subject: [PATCH 7/9] Switch to abstract class --- ...iliosmssender_twilioverificationsender.py} | 40 +++++++------------ engine/apps/twilioapp/models/twilio_sender.py | 10 +++-- engine/apps/twilioapp/phone_provider.py | 15 +++---- 3 files changed, 26 insertions(+), 39 deletions(-) rename engine/apps/twilioapp/migrations/{0005_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py => 0005_twilioaccount_twiliophonecallsender_twiliosmssender_twilioverificationsender.py} (56%) diff --git a/engine/apps/twilioapp/migrations/0005_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py b/engine/apps/twilioapp/migrations/0005_twilioaccount_twiliophonecallsender_twiliosmssender_twilioverificationsender.py similarity index 56% rename from engine/apps/twilioapp/migrations/0005_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py rename to engine/apps/twilioapp/migrations/0005_twilioaccount_twiliophonecallsender_twiliosmssender_twilioverificationsender.py index 0f4ecd8ca3..2439a07c8b 100644 --- a/engine/apps/twilioapp/migrations/0005_twilioaccount_twiliophonecallsender_twiliosender_twiliosmssender_twilioverificationsender.py +++ b/engine/apps/twilioapp/migrations/0005_twilioaccount_twiliophonecallsender_twiliosmssender_twilioverificationsender.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.19 on 2023-05-24 19:30 +# Generated by Django 3.2.19 on 2023-05-25 15:32 from django.db import migrations, models import django.db.models.deletion @@ -7,7 +7,6 @@ class Migration(migrations.Migration): dependencies = [ - ('contenttypes', '0002_remove_content_type_name'), ('twilioapp', '0004_twiliophonecall_twiliosms'), ] @@ -24,53 +23,42 @@ class Migration(migrations.Migration): ], ), migrations.CreateModel( - name='TwilioSender', + name='TwilioVerificationSender', fields=[ ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('name', models.CharField(default='Default', max_length=100)), ('country_code', models.CharField(default=None, max_length=16, null=True)), - ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='twilio_sender', to='twilioapp.twilioaccount')), - ('polymorphic_ctype', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='polymorphic_twilioapp.twiliosender_set+', to='contenttypes.contenttype')), - ], - options={ - 'abstract': False, - 'base_manager_name': 'objects', - }, - ), - migrations.CreateModel( - name='TwilioPhoneCallSender', - fields=[ - ('twiliosender_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='twilioapp.twiliosender')), - ('number', models.CharField(max_length=16)), + ('verify_service_sid', models.CharField(max_length=64)), + ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='twilioapp_twilioverificationsender_account', to='twilioapp.twilioaccount')), ], options={ 'abstract': False, - 'base_manager_name': 'objects', }, - bases=('twilioapp.twiliosender',), ), migrations.CreateModel( name='TwilioSmsSender', fields=[ - ('twiliosender_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='twilioapp.twiliosender')), + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(default='Default', max_length=100)), + ('country_code', models.CharField(default=None, max_length=16, null=True)), ('sender', models.CharField(max_length=16)), + ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='twilioapp_twiliosmssender_account', to='twilioapp.twilioaccount')), ], options={ 'abstract': False, - 'base_manager_name': 'objects', }, - bases=('twilioapp.twiliosender',), ), migrations.CreateModel( - name='TwilioVerificationSender', + name='TwilioPhoneCallSender', fields=[ - ('twiliosender_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='twilioapp.twiliosender')), - ('verify_service_sid', models.CharField(max_length=64)), + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(default='Default', max_length=100)), + ('country_code', models.CharField(default=None, max_length=16, null=True)), + ('number', models.CharField(max_length=16)), + ('account', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='twilioapp_twiliophonecallsender_account', to='twilioapp.twilioaccount')), ], options={ 'abstract': False, - 'base_manager_name': 'objects', }, - bases=('twilioapp.twiliosender',), ), ] diff --git a/engine/apps/twilioapp/models/twilio_sender.py b/engine/apps/twilioapp/models/twilio_sender.py index a951895a7a..c18d33f0f7 100644 --- a/engine/apps/twilioapp/models/twilio_sender.py +++ b/engine/apps/twilioapp/models/twilio_sender.py @@ -1,5 +1,4 @@ from django.db import models -from polymorphic.models import PolymorphicModel from twilio.rest import Client @@ -17,11 +16,16 @@ def get_twilio_api_client(self): return Client(self.account_sid, self.auth_token) -class TwilioSender(PolymorphicModel): +class TwilioSender(models.Model): name = models.CharField(max_length=100, null=False, default="Default") # Note: country_code does not have + prefix here country_code = models.CharField(max_length=16, null=True, default=None) - account = models.ForeignKey("twilioapp.TwilioAccount", on_delete=models.CASCADE, related_name="twilio_sender") + account = models.ForeignKey( + "twilioapp.TwilioAccount", on_delete=models.CASCADE, related_name="%(app_label)s_%(class)s_account" + ) + + class Meta: + abstract = True class TwilioSmsSender(TwilioSender): diff --git a/engine/apps/twilioapp/phone_provider.py b/engine/apps/twilioapp/phone_provider.py index 84d8db5dd9..70e1e8c318 100644 --- a/engine/apps/twilioapp/phone_provider.py +++ b/engine/apps/twilioapp/phone_provider.py @@ -19,7 +19,6 @@ from apps.phone_notifications.phone_provider import PhoneProvider, ProviderFlags from apps.twilioapp.gather import get_gather_message, get_gather_url from apps.twilioapp.models import TwilioCallStatuses, TwilioPhoneCall, TwilioSMS -from apps.twilioapp.models.twilio_sender import TwilioPhoneCallSender, TwilioSmsSender, TwilioVerificationSender from apps.twilioapp.status_callback import get_call_status_callback_url, get_sms_status_callback_url logger = logging.getLogger(__name__) @@ -234,12 +233,8 @@ def _default_twilio_number(self): def _twilio_sender(self, sender_type, to): _, _, country_code = self._parse_number(to) - TwilioSender = apps.get_model("twilioapp", "TwilioSender") - senders = list( - TwilioSender.objects.instance_of(sender_type).filter( - Q(country_code=country_code) | Q(country_code__isnull=True) - ) - ) + TwilioSender = apps.get_model("twilioapp", sender_type) + senders = list(TwilioSender.objects.filter(Q(country_code=country_code) | Q(country_code__isnull=True))) senders.sort(key=lambda x: (not x.country_code, x)) if senders: @@ -248,15 +243,15 @@ def _twilio_sender(self, sender_type, to): return self._default_twilio_api_client, None def _sms_sender(self, to): - client, sender = self._twilio_sender(TwilioSmsSender, to) + client, sender = self._twilio_sender("TwilioSmsSender", to) return client, sender.sender if sender else self._default_twilio_number def _phone_sender(self, to): - client, sender = self._twilio_sender(TwilioPhoneCallSender, to) + client, sender = self._twilio_sender("TwilioPhoneCallSender", to) return client, sender.number if sender else self._default_twilio_number def _verify_sender(self, to): - client, sender = self._twilio_sender(TwilioVerificationSender, to) + client, sender = self._twilio_sender("TwilioVerificationSender", to) return client, sender.verify_service_sid if sender else live_settings.TWILIO_VERIFY_SERVICE_SID def _get_calling_code(self, iso): From 469154c6a3db813f524c9ce23a1f91a2875f0b71 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 25 May 2023 11:17:25 -0600 Subject: [PATCH 8/9] Fix CHANGELOG --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5573e03d5e..86d6df2c39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,15 @@ the destination country code by @mderynck ([#1976](https://github.com/grafana/on ## v1.2.29 (2023-05-25) +### Changed + +- Phone provider refactoring [#1713](https://github.com/grafana/oncall/pull/1713) + +### Fixed + +- Handle slack metadata limit when creating paging command payload ([#2007](https://github.com/grafana/oncall/pull/2007)) +- Fix issue with sometimes cached final schedule not being refreshed after an update ([#2004](https://github.com/grafana/oncall/pull/2004)) + ## v1.2.28 (2023-05-24) ### Fixed From fab62972c4f539e255169ae5a39ab140ce0c6783 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 25 May 2023 15:30:26 -0600 Subject: [PATCH 9/9] Switch back to order by + nulls_last --- engine/apps/twilioapp/phone_provider.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/engine/apps/twilioapp/phone_provider.py b/engine/apps/twilioapp/phone_provider.py index 70e1e8c318..87a6450f2c 100644 --- a/engine/apps/twilioapp/phone_provider.py +++ b/engine/apps/twilioapp/phone_provider.py @@ -3,7 +3,7 @@ from string import digits from django.apps import apps -from django.db.models import Q +from django.db.models import F, Q from phonenumbers import COUNTRY_CODE_TO_REGION_CODE from twilio.base.exceptions import TwilioRestException from twilio.rest import Client @@ -234,11 +234,14 @@ def _default_twilio_number(self): def _twilio_sender(self, sender_type, to): _, _, country_code = self._parse_number(to) TwilioSender = apps.get_model("twilioapp", sender_type) - senders = list(TwilioSender.objects.filter(Q(country_code=country_code) | Q(country_code__isnull=True))) - senders.sort(key=lambda x: (not x.country_code, x)) + sender = ( + TwilioSender.objects.filter(Q(country_code=country_code) | Q(country_code__isnull=True)) + .order_by(F("country_code").desc(nulls_last=True)) + .first() + ) - if senders: - return senders[0].account.get_twilio_api_client(), senders[0] + if sender: + return sender.account.get_twilio_api_client(), sender return self._default_twilio_api_client, None