From c1501e09bf2bf20037443164b4c4486302d0b6c1 Mon Sep 17 00:00:00 2001 From: Steve Astels Date: Wed, 17 Jul 2024 09:59:57 -0400 Subject: [PATCH] Use Pinpoint and default sender id for international sending (#2221) --- app/clients/sms/aws_pinpoint.py | 25 ++++++++++++------ app/delivery/send_to_providers.py | 5 ++-- tests/app/clients/test_aws_pinpoint.py | 27 ++++++++++++++++++++ tests/app/delivery/test_send_to_providers.py | 7 ++--- 4 files changed, 50 insertions(+), 14 deletions(-) diff --git a/app/clients/sms/aws_pinpoint.py b/app/clients/sms/aws_pinpoint.py index 7007b867a6..4b18b1c7a3 100644 --- a/app/clients/sms/aws_pinpoint.py +++ b/app/clients/sms/aws_pinpoint.py @@ -41,16 +41,25 @@ def send_sms(self, to, content, reference, multi=True, sender=None, template_id= opted_out = False to = phonenumbers.format_number(match.number, phonenumbers.PhoneNumberFormat.E164) destinationNumber = to - try: start_time = monotonic() - response = self._client.send_text_message( - DestinationPhoneNumber=destinationNumber, - OriginationIdentity=pool_id, - MessageBody=content, - MessageType=messageType, - ConfigurationSetName=self.current_app.config["AWS_PINPOINT_CONFIGURATION_SET_NAME"], - ) + # For international numbers we send with an AWS number for the corresponding country, using our default sender id. + # Note that Canada does not currently support sender ids. + if phonenumbers.region_code_for_number(match.number) != "CA": + response = self._client.send_text_message( + DestinationPhoneNumber=destinationNumber, + MessageBody=content, + MessageType=messageType, + ConfigurationSetName=self.current_app.config["AWS_PINPOINT_CONFIGURATION_SET_NAME"], + ) + else: + response = self._client.send_text_message( + DestinationPhoneNumber=destinationNumber, + OriginationIdentity=pool_id, + MessageBody=content, + MessageType=messageType, + ConfigurationSetName=self.current_app.config["AWS_PINPOINT_CONFIGURATION_SET_NAME"], + ) except self._client.exceptions.ConflictException as e: if e.response.get("Reason") == "DESTINATION_PHONE_NUMBER_OPTED_OUT": opted_out = True diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index c470586edd..5979517c75 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -395,13 +395,12 @@ def provider_to_use( elif phonenumbers.region_code_for_number(match.number) != "CA": recipient_outside_canada = True using_sc_pool_template = template_id is not None and str(template_id) in current_app.config["AWS_PINPOINT_SC_TEMPLATE_IDS"] - + zone_1_outside_canada = recipient_outside_canada and not international do_not_use_pinpoint = ( has_dedicated_number or sending_to_us_number or cannot_determine_recipient_country - or international - or recipient_outside_canada + or zone_1_outside_canada or not current_app.config["AWS_PINPOINT_SC_POOL_ID"] or ((not current_app.config["AWS_PINPOINT_DEFAULT_POOL_ID"]) and not using_sc_pool_template) ) diff --git a/tests/app/clients/test_aws_pinpoint.py b/tests/app/clients/test_aws_pinpoint.py index f73692f23d..b681025c1b 100644 --- a/tests/app/clients/test_aws_pinpoint.py +++ b/tests/app/clients/test_aws_pinpoint.py @@ -112,3 +112,30 @@ def test_handles_opted_out_numbers(notify_api, mocker, sample_template): content = "foo" reference = "ref" assert aws_pinpoint_client.send_sms(to, content, reference=reference, template_id=sample_template.id) == "opted_out" + + +@pytest.mark.serial +def test_send_sms_sends_international_without_pool_id(notify_api, mocker, sample_template): + boto_mock = mocker.patch.object(aws_pinpoint_client, "_client", create=True) + mocker.patch.object(aws_pinpoint_client, "statsd_client", create=True) + to = "+447512501324" + content = "foo" + reference = "ref" + + with set_config_values( + notify_api, + { + "AWS_PINPOINT_SC_POOL_ID": "sc_pool_id", + "AWS_PINPOINT_DEFAULT_POOL_ID": "default_pool_id", + "AWS_PINPOINT_CONFIGURATION_SET_NAME": "config_set_name", + "AWS_PINPOINT_SC_TEMPLATE_IDS": [], + }, + ): + aws_pinpoint_client.send_sms(to, content, reference=reference, template_id=sample_template.id) + + boto_mock.send_text_message.assert_called_once_with( + DestinationPhoneNumber="+447512501324", + MessageBody=content, + MessageType="TRANSACTIONAL", + ConfigurationSetName="config_set_name", + ) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index ed293c0b78..d0c0d6812e 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -110,7 +110,8 @@ def test_should_use_sns_for_sms_if_sending_to_the_US(self, restore_provider_deta provider = send_to_providers.provider_to_use("sms", "1234", "+17065551234") assert provider.name == "sns" - def test_should_use_sns_for_sms_if_sending_outside_zone_1(self, restore_provider_details, notify_api): + @pytest.mark.serial + def test_should_use_pinpoint_for_sms_if_sending_outside_zone_1(self, restore_provider_details, notify_api): with set_config_values( notify_api, { @@ -118,8 +119,8 @@ def test_should_use_sns_for_sms_if_sending_outside_zone_1(self, restore_provider "AWS_PINPOINT_DEFAULT_POOL_ID": "default_pool_id", }, ): - provider = send_to_providers.provider_to_use("sms", "1234", "+17065551234", international=True) - assert provider.name == "sns" + provider = send_to_providers.provider_to_use("sms", "1234", "+447512501324", international=True) + assert provider.name == "pinpoint" def test_should_use_sns_for_sms_if_sending_to_non_CA_zone_1(self, restore_provider_details, notify_api): with set_config_values(