Merge pull request #3161 from alphagov/check-use_numeric_sender

Check international rules to decide on a sender

app/user/rest.py

@@ -4,7 +4,7 @@ from datetime import datetime
 from urllib.parse import urlencode
 
 from flask import (jsonify, request, Blueprint, current_app, abort)
-from notifications_utils.recipients import is_uk_phone_number
+from notifications_utils.recipients import is_uk_phone_number, use_numeric_sender
 from sqlalchemy.exc import IntegrityError
 
 from app.config import QueueNames
@@ -105,10 +105,7 @@ def update_user_attribute(user_id):
         elif 'mobile_number' in update_dct:
             template = dao_get_template_by_id(current_app.config['TEAM_MEMBER_EDIT_MOBILE_TEMPLATE_ID'])
             recipient = user_to_update.mobile_number
-            if is_uk_phone_number(recipient):
-                reply_to = template.service.get_default_sms_sender()
-            else:
-                reply_to = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER']
+            reply_to = get_sms_reply_to_for_notify_service(recipient, template)
         else:
             return jsonify(data=user_to_update.serialize()), 200
         service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID'])
@@ -133,6 +130,14 @@ def update_user_attribute(user_id):
     return jsonify(data=user_to_update.serialize()), 200
 
 
+def get_sms_reply_to_for_notify_service(recipient, template):
+    if not is_uk_phone_number(recipient) and use_numeric_sender(recipient):
+        reply_to = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER']
+    else:
+        reply_to = template.service.get_default_sms_sender()
+    return reply_to
+
+
 @user_blueprint.route('/<uuid:user_id>/archive', methods=['POST'])
 def archive_user(user_id):
     user = get_user_by_id(user_id)
@@ -271,10 +276,7 @@ def create_2fa_code(template_id, user_to_send_to, secret_code, recipient, person
     create_user_code(user_to_send_to, secret_code, template.template_type)
     reply_to = None
     if template.template_type == SMS_TYPE:
-        if is_uk_phone_number(recipient):
-            reply_to = template.service.get_default_sms_sender()
-        else:
-            reply_to = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER']
+        reply_to = get_sms_reply_to_for_notify_service(recipient, template)
     elif template.template_type == EMAIL_TYPE:
         reply_to = template.service.get_default_reply_to_email_address()
     saved_notification = persist_notification(

requirements-app.txt

@@ -33,7 +33,7 @@ notifications-python-client==5.7.1
 # PaaS
 awscli-cwlogs==1.4.6
 
-git+https://github.com/alphagov/notifications-utils.git@43.9.0#egg=notifications-utils==43.9.0
+git+https://github.com/alphagov/notifications-utils.git@43.9.1#egg=notifications-utils==43.9.1
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.9.0

requirements.txt

@@ -35,25 +35,25 @@ notifications-python-client==5.7.1
 # PaaS
 awscli-cwlogs==1.4.6
 
-git+https://github.com/alphagov/notifications-utils.git@43.9.0#egg=notifications-utils==43.9.0
+git+https://github.com/alphagov/notifications-utils.git@43.9.1#egg=notifications-utils==43.9.1
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.9.0
 gds-metrics==0.2.4
 
 ## The following requirements were added by pip freeze:
-alembic==1.5.4
+alembic==1.5.5
 amqp==1.4.9
 anyjson==0.3.3
 attrs==20.3.0
-awscli==1.19.8
+awscli==1.19.14
 bcrypt==3.2.0
 billiard==3.3.0.23
 bleach==3.3.0
 blinker==1.4
 boto==2.49.0
-boto3==1.17.8
-botocore==1.20.8
+boto3==1.17.14
+botocore==1.20.14
 certifi==2020.12.5
 chardet==4.0.0
 click==7.1.2
@@ -66,7 +66,7 @@ geojson==2.5.0
 govuk-bank-holidays==0.8
 greenlet==1.0.0
 idna==2.10
-importlib-metadata==3.4.0
+importlib-metadata==3.6.0
 Jinja2==2.11.3
 jmespath==0.10.0
 kombu==3.0.37
@@ -86,10 +86,10 @@ python-dateutil==2.8.1
 python-editor==1.0.4
 python-json-logger==2.0.1
 pytz==2021.1
-PyYAML==5.3.1
+PyYAML==5.4.1
 redis==3.5.3
 requests==2.25.1
-rsa==4.5
+rsa==4.7.2
 s3transfer==0.3.4
 Shapely==1.7.1
 six==1.15.0

tests/app/user/test_rest.py

@@ -315,7 +315,7 @@ def test_post_user_attribute_with_updated_by_sends_notification_to_international
 ):
     updater = create_user(name="Service Manago")
     update_dict = {
-        'mobile_number': '601117224412',
+        'mobile_number': '+601117224412',
         'updated_by': str(updater.id)
     }
     auth_header = create_authorization_header()
@@ -1086,3 +1086,20 @@ def test_search_for_users_by_email_handles_incorrect_data_format(notify_db, clie
 
     assert response.status_code == 400
     assert json.loads(response.get_data(as_text=True))['message'] == {'email': ['Not a valid string.']}
+
+
+@pytest.mark.parametrize('number, expected_reply_to',
+                         [
+                          ("1-403-123-4567", "notify_international_sender"),
+                          ("27 123 4569 2312", "notify_international_sender"),
+                          ("30 123 4567 7890", "Notify"),
+                          ("+20 123 4567 7890", "Notify"),
+                          ])
+def test_get_sms_reply_to_for_notify_service(team_member_mobile_edit_template, number, expected_reply_to):
+    # need to import locally to avoid db session errors,
+    # if this import is with the other imports at the top of the file
+    # the imports happen in the wrong order and you'll see "dummy session" errors
+    from app.user.rest import get_sms_reply_to_for_notify_service
+    reply_to = get_sms_reply_to_for_notify_service(number, team_member_mobile_edit_template)
+    assert reply_to == current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'] \
+        if expected_reply_to == 'notify_international_sender' else current_app.config['FROM_NUMBER']