diff --git a/app/user/rest.py b/app/user/rest.py index 32aa7b532..6867e58b6 100644 --- a/app/user/rest.py +++ b/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('//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( diff --git a/requirements-app.txt b/requirements-app.txt index 8d99c0d8f..80933ce40 100644 --- a/requirements-app.txt +++ b/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 diff --git a/requirements.txt b/requirements.txt index 41242987a..6bedf5a41 100644 --- a/requirements.txt +++ b/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 diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 64ad80d15..07fa440c2 100644 --- a/tests/app/user/test_rest.py +++ b/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']