diff --git a/app/config.py b/app/config.py index b3528cb49..4bdec85b6 100644 --- a/app/config.py +++ b/app/config.py @@ -287,7 +287,7 @@ class Live(Config): NOTIFY_ENVIRONMENT = 'live' CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' STATSD_ENABLED = True - FROM_NUMBER = '40604' + FROM_NUMBER = 'GOVUK' FUNCTIONAL_TEST_PROVIDER_SERVICE_ID = '6c1d81bb-dae2-4ee9-80b0-89a4aae9f649' FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID = 'ba9e1789-a804-40b8-871f-cc60d4c1286f' PERFORMANCE_PLATFORM_ENABLED = True diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 0a9aa22f0..df4c664f9 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -23,6 +23,7 @@ from app.celery.statistics_tasks import record_initial_job_statistics, create_in def send_sms_to_provider(notification): service = notification.service + if not service.active: technical_failure(notification=notification) return @@ -37,7 +38,7 @@ def send_sms_to_provider(notification): template_model.__dict__, values=notification.personalisation, prefix=service.name, - sender=service.sms_sender + sender=service.sms_sender not in {None, current_app.config['FROM_NUMBER']} ) if service.research_mode or notification.key_type == KEY_TYPE_TEST: @@ -50,7 +51,7 @@ def send_sms_to_provider(notification): to=validate_and_format_phone_number(notification.to, international=notification.international), content=str(template), reference=str(notification.id), - sender=service.sms_sender + sender=service.sms_sender or current_app.config['FROM_NUMBER'] ) except Exception as e: dao_toggle_sms_provider(provider.name) diff --git a/app/models.py b/app/models.py index 378c45b3e..8b3b8e053 100644 --- a/app/models.py +++ b/app/models.py @@ -188,7 +188,7 @@ class Service(db.Model, Versioned): created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) reply_to_email_address = db.Column(db.Text, index=False, unique=False, nullable=True) letter_contact_block = db.Column(db.Text, index=False, unique=False, nullable=True) - sms_sender = db.Column(db.String(11), nullable=True) + sms_sender = db.Column(db.String(11), nullable=True, default=lambda: current_app.config['FROM_NUMBER']) organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) organisation = db.relationship('Organisation') dvla_organisation_id = db.Column( diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 1d243b3e5..5cb467ac3 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -5,6 +5,7 @@ from unittest.mock import ANY, call import pytest from notifications_utils.recipients import validate_and_format_phone_number +from flask import current_app import app from app import mmg_client, firetext_client @@ -73,7 +74,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( to=validate_and_format_phone_number("+447234123123"), content="Sample service: Hello Jo\nHere is some HTML & entities", reference=str(db_notification.id), - sender=None + sender=current_app.config['FROM_NUMBER'] ) stats_mock.assert_called_once_with(db_notification) @@ -175,7 +176,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( to=validate_and_format_phone_number("+447234123123"), content="Sample service: This is a template:\nwith a newline", reference=str(db_notification.id), - sender=None + sender=current_app.config['FROM_NUMBER'] ) persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) @@ -549,7 +550,7 @@ def test_should_send_sms_to_international_providers( to="447234123999", content=ANY, reference=str(db_notification_uk.id), - sender=None + sender=current_app.config['FROM_NUMBER'] ) send_to_providers.send_sms_to_provider( @@ -560,7 +561,7 @@ def test_should_send_sms_to_international_providers( to="447234123111", content=ANY, reference=str(db_notification_international.id), - sender=None + sender=current_app.config['FROM_NUMBER'] ) notification_uk = Notification.query.filter_by(id=db_notification_uk.id).one() @@ -619,3 +620,34 @@ def test_should_set_international_phone_number_to_sent_status( ) assert notification.status == 'sent' + + +@pytest.mark.parametrize('sms_sender, expected_sender, expected_content', [ + ('foo', 'foo', 'bar'), + # if 40604 is actually in DB then treat that as if entered manually + ('40604', '40604', 'bar'), + # 'testing' is the FROM_NUMBER during unit tests + (None, 'testing', 'Sample service: bar'), + ('testing', 'testing', 'Sample service: bar'), +]) +def test_should_handle_sms_sender_and_prefix_message( + sample_service, + mocker, + sms_sender, + expected_sender, + expected_content +): + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') + sample_service.sms_sender = sms_sender + template = create_template(sample_service, content='bar') + notification = create_notification(template) + + send_to_providers.send_sms_to_provider(notification) + + mmg_client.send_sms.assert_called_once_with( + content=expected_content, + sender=expected_sender, + to=ANY, + reference=ANY, + ) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index c12ca4eec..126e9b948 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -5,7 +5,7 @@ import uuid from unittest.mock import ANY import pytest -from flask import url_for +from flask import url_for, current_app from freezegun import freeze_time from app.dao.users_dao import save_model_user @@ -144,6 +144,7 @@ def test_get_service_by_id(client, sample_service): assert json_resp['data']['organisation'] is None assert json_resp['data']['branding'] == 'govuk' assert json_resp['data']['dvla_organisation'] == '001' + assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): @@ -213,6 +214,7 @@ def test_create_service(client, sample_user): assert json_resp['data']['email_from'] == 'created.service' assert not json_resp['data']['research_mode'] assert json_resp['data']['dvla_organisation'] == '001' + assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] auth_header_fetch = create_authorization_header() diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index 3223e9e71..cb6ee93fd 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -109,5 +109,4 @@ def test_send_notification_to_service_users_sends_to_active_users_only( assert Notification.query.count() == 2 - assert notifications[0].to == first_active_user.email_address - assert notifications[1].to == second_active_user.email_address + assert pending_user.email_address not in [n.to for n in notifications] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 14f8d6d2e..4e721f15f 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1,6 +1,8 @@ import uuid + import pytest -from flask import json +from flask import json, current_app + from app.models import Notification from app.v2.errors import RateLimitError from tests import create_authorization_header @@ -33,8 +35,7 @@ def test_post_sms_notification_returns_201(notify_api, sample_template_with_plac assert resp_json['id'] == str(notification_id) assert resp_json['reference'] == reference assert resp_json['content']['body'] == sample_template_with_placeholders.content.replace("(( Name))", "Jo") - # conftest fixture service does not have a sms sender, use config default - assert resp_json['content']['from_number'] == notify_api.config["FROM_NUMBER"] + assert resp_json['content']['from_number'] == current_app.config['FROM_NUMBER'] assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri'] assert resp_json['template']['id'] == str(sample_template_with_placeholders.id) assert resp_json['template']['version'] == sample_template_with_placeholders.version