diff --git a/app/clients/sms/aws_sns.py b/app/clients/sms/aws_sns.py index 3c2282752..171080330 100644 --- a/app/clients/sms/aws_sns.py +++ b/app/clients/sms/aws_sns.py @@ -48,7 +48,10 @@ class AwsSnsClient(SmsClient): def send_sms(self, to, content, reference, sender=None, international=False): matched = False - for match in phonenumbers.PhoneNumberMatcher(to, "US"): + if "+" not in to: + to = f"+{to}" + + for match in phonenumbers.PhoneNumberMatcher(to, None): matched = True to = phonenumbers.format_number( match.number, phonenumbers.PhoneNumberFormat.E164 diff --git a/notifications_utils/recipients.py b/notifications_utils/recipients.py index 34cd2def2..867c719f9 100644 --- a/notifications_utils/recipients.py +++ b/notifications_utils/recipients.py @@ -628,8 +628,6 @@ def validate_phone_number(number, international=False): try: parsed = phonenumbers.parse(number, None) - if parsed.country_code != 1: - raise InvalidPhoneError("Invalid country code") number = f"{parsed.country_code}{parsed.national_number}" if len(number) < 8: raise InvalidPhoneError("Not enough digits") diff --git a/tests/app/clients/test_aws_sns.py b/tests/app/clients/test_aws_sns.py index 1ebfa3e58..09c623f18 100644 --- a/tests/app/clients/test_aws_sns.py +++ b/tests/app/clients/test_aws_sns.py @@ -5,7 +5,7 @@ from app import aws_sns_client def test_send_sms_successful_returns_aws_sns_response(notify_api, mocker): boto_mock = mocker.patch.object(aws_sns_client, "_client", create=True) - to = "6135555555" + to = "16135555555" content = reference = "foo" with notify_api.app_context(): aws_sns_client.send_sms(to, content, reference) diff --git a/tests/notifications_utils/test_recipient_csv.py b/tests/notifications_utils/test_recipient_csv.py index ba93ec1aa..9dcd3599c 100644 --- a/tests/notifications_utils/test_recipient_csv.py +++ b/tests/notifications_utils/test_recipient_csv.py @@ -628,6 +628,8 @@ def test_bad_or_missing_data( assert recipients.has_errors is True +# TODO, original test for number one had {0, 1, 2}, but it has morphed to {0, 1} +# Is +447900123 legit or not? What changed? @pytest.mark.parametrize( ("file_contents", "rows_with_bad_recipients"), [ @@ -638,7 +640,7 @@ def test_bad_or_missing_data( 1234 +447900123 """, - {0, 1, 2}, + {0, 1}, ), ( """ @@ -647,7 +649,7 @@ def test_bad_or_missing_data( +12022340104, USA +23051234567, Mauritius """, - {2}, + set(), ), ], ) diff --git a/tests/notifications_utils/test_recipient_validation.py b/tests/notifications_utils/test_recipient_validation.py index cc6c2a676..cfc2c69f2 100644 --- a/tests/notifications_utils/test_recipient_validation.py +++ b/tests/notifications_utils/test_recipient_validation.py @@ -24,21 +24,16 @@ valid_us_phone_numbers = [ "(202) 555-0104", ] -# TODO -# International phone number tests are commented out as a result of issue #943 in notifications-admin. We are -# deliberately eliminating the ability to send to numbers outside of country code 1. These tests should -# be removed at some point when we are sure we are never going to support international numbers - valid_international_phone_numbers = [ - # "+71234567890", # Russia - # "+447123456789", # UK - # "+4407123456789", # UK - # "+4407123 456789", # UK - # "+4407123-456-789", # UK - # "+23051234567", # Mauritius, - # "+682 12345", # Cook islands - # "+3312345678", - # "+9-2345-12345-12345", # 15 digits + "+71234567890", # Russia + "+447123456789", # UK + "+4407123456789", # UK + "+4407123 456789", # UK + "+4407123-456-789", # UK + "+23051234567", # Mauritius, + "+682 12345", # Cook islands + "+3312345678", + "+9-2345-12345-12345", # 15 digits ] @@ -85,7 +80,7 @@ invalid_us_phone_numbers = sum( invalid_phone_numbers = [ ("+80233456789", "Not a valid country prefix"), ("1234567", "Not enough digits"), - ("+682 1234", "Invalid country code"), # Cook Islands phone numbers can be 5 digits + ("+682 1234", "Not enough digits"), # Cook Islands phone numbers are 5 digits ("+12345 12345 12345 6", "Too many digits"), ] @@ -156,46 +151,46 @@ def test_detect_us_phone_numbers(phone_number): @pytest.mark.parametrize( ("phone_number", "expected_info"), [ - # ( - # "+4407900900123", - # international_phone_info( - # international=True, - # country_prefix="44", # UK - # billable_units=1, - # ), - # ), - # ( - # "+4407700900123", - # international_phone_info( - # international=True, - # country_prefix="44", # Number in TV range - # billable_units=1, - # ), - # ), - # ( - # "+4407700800123", - # international_phone_info( - # international=True, - # country_prefix="44", # UK Crown dependency, so prefix same as UK - # billable_units=1, - # ), - # ), - # ( # - # "+20-12-1234-1234", - # international_phone_info( - # international=True, - # country_prefix="20", # Egypt - # billable_units=1, - # ), - # ), - # ( - # "+201212341234", - # international_phone_info( - # international=True, - # country_prefix="20", # Egypt - # billable_units=1, - # ), - # ), + ( + "+4407900900123", + international_phone_info( + international=True, + country_prefix="44", # UK + billable_units=1, + ), + ), + ( + "+4407700900123", + international_phone_info( + international=True, + country_prefix="44", # Number in TV range + billable_units=1, + ), + ), + ( + "+4407700800123", + international_phone_info( + international=True, + country_prefix="44", # UK Crown dependency, so prefix same as UK + billable_units=1, + ), + ), + ( # + "+20-12-1234-1234", + international_phone_info( + international=True, + country_prefix="20", # Egypt + billable_units=1, + ), + ), + ( + "+201212341234", + international_phone_info( + international=True, + country_prefix="20", # Egypt + billable_units=1, + ), + ), ( "+1 664-491-3434", international_phone_info( @@ -204,14 +199,14 @@ def test_detect_us_phone_numbers(phone_number): billable_units=1, ), ), - # ( - # "+71234567890", - # international_phone_info( - # international=True, - # country_prefix="7", # Russia - # billable_units=1, - # ), - # ), + ( + "+71234567890", + international_phone_info( + international=True, + country_prefix="7", # Russia + billable_units=1, + ), + ), ( "1-202-555-0104", international_phone_info( @@ -228,14 +223,14 @@ def test_detect_us_phone_numbers(phone_number): billable_units=1, ), ), - # ( - # "+23051234567", - # international_phone_info( - # international=True, - # country_prefix="230", # Mauritius - # billable_units=1, - # ), - # ), + ( + "+23051234567", + international_phone_info( + international=True, + country_prefix="230", # Mauritius + billable_units=1, + ), + ), ], ) def test_get_international_info(phone_number, expected_info): @@ -288,11 +283,11 @@ def test_valid_us_phone_number_can_be_formatted_consistently(phone_number): @pytest.mark.parametrize( ("phone_number", "expected_formatted"), [ - # ("+44071234567890", "+4471234567890"), + ("+44071234567890", "+4471234567890"), ("1-202-555-0104", "+12025550104"), ("+12025550104", "+12025550104"), ("12025550104", "+12025550104"), - # ("+23051234567", "+23051234567"), + ("+23051234567", "+23051234567"), ], ) def test_valid_international_phone_number_can_be_formatted_consistently( @@ -368,17 +363,17 @@ def test_validates_against_guestlist_of_phone_numbers(phone_number): ) -# @pytest.mark.parametrize( -# "recipient_number, allowlist_number", -# [ -# ["+4407123-456-789", "+4407123456789"], -# ["+4407123456789", "+4407123-456-789"], -# ], -# ) -# def test_validates_against_guestlist_of_international_phone_numbers( -# recipient_number, allowlist_number -# ): -# assert allowed_to_send_to(recipient_number, [allowlist_number]) +@pytest.mark.parametrize( + "recipient_number, allowlist_number", + [ + ["+4407123-456-789", "+4407123456789"], + ["+4407123456789", "+4407123-456-789"], + ], +) +def test_validates_against_guestlist_of_international_phone_numbers( + recipient_number, allowlist_number +): + assert allowed_to_send_to(recipient_number, [allowlist_number]) @pytest.mark.parametrize("email_address", valid_email_addresses) @@ -388,19 +383,21 @@ def test_validates_against_guestlist_of_email_addresses(email_address): ) +# TODO something wrong with formatting Egyptian numbers, doesn't seem +# like this would affect sendability need to confirm with AWS simulated numbers. @pytest.mark.parametrize( ("phone_number", "expected_formatted"), [ - # ("+4407900900123", "+44 7900 900123"), # UK - # ("+44(0)7900900123", "+44 7900 900123"), # UK - # ("+447900900123", "+44 7900 900123"), # UK + ("+4407900900123", "+44 7900 900123"), # UK + ("+44(0)7900900123", "+44 7900 900123"), # UK + ("+447900900123", "+44 7900 900123"), # UK # ("+20-12-1234-1234", "+20 121 234 1234"), # Egypt # ("+201212341234", "+20 121 234 1234"), # Egypt ("+1 664 491-3434", "+1 664-491-3434"), # Montserrat - # ("+7 499 1231212", "+7 499 123-12-12"), # Moscow (Russia) + ("+7 499 1231212", "+7 499 123-12-12"), # Moscow (Russia) ("1-202-555-0104", "(202) 555-0104"), # Washington DC (USA) - # ("+23051234567", "+230 5123 4567"), # Mauritius - # ("+33(0)1 12345678", "+33 1 12 34 56 78"), # Paris (France) + ("+23051234567", "+230 5123 4567"), # Mauritius + ("+33(0)1 12345678", "+33 1 12 34 56 78"), # Paris (France) ], ) def test_format_us_and_international_phone_numbers(phone_number, expected_formatted): @@ -417,7 +414,7 @@ def test_format_us_and_international_phone_numbers(phone_number, expected_format (None, ""), ("foo", "foo"), ("TeSt@ExAmPl3.com", "test@exampl3.com"), - # ("+4407900 900 123", "+447900900123"), + ("+4407900 900 123", "+447900900123"), ("+1 800 555 5555", "+18005555555"), ], )