Merge pull request #973 from alphagov/40604-or-govuk

treat 40604 and GOVUK as not having a sender ID

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 <em>some HTML</em> & 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,
+    )

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()
 

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]

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