From 812090b1048b2c7a61d4fc7c299200f094549853 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 3 Nov 2017 15:32:18 +0000 Subject: [PATCH] Default new services to prefixing text messages We want new services, when they do the tour, to see how the service name they just made shows up in the messages. This is how it (should) work at the moment (although got broken because of the multiple senders stuff). Need to do this before we do the migration now otherwise a new service could sneak in with this setting still set to `null`. --- app/models.py | 2 +- tests/app/dao/test_services_dao.py | 1 + tests/app/delivery/test_send_to_providers.py | 17 ++++++++++------- tests/app/service/test_rest.py | 10 +++++----- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/app/models.py b/app/models.py index 6037bd26d..74b0af79a 100644 --- a/app/models.py +++ b/app/models.py @@ -224,7 +224,7 @@ class Service(db.Model, Versioned): _reply_to_email_address = db.Column("reply_to_email_address", db.Text, index=False, unique=False, nullable=True) _letter_contact_block = db.Column('letter_contact_block', db.Text, index=False, unique=False, nullable=True) sms_sender = db.Column(db.String(11), nullable=False, default=lambda: current_app.config['FROM_NUMBER']) - prefix_sms = db.Column(db.Boolean, nullable=True) + prefix_sms = db.Column(db.Boolean, nullable=True, default=True) organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) free_sms_fragment_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=True) organisation = db.relationship('Organisation') diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 76c06d088..c45ce0272 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -93,6 +93,7 @@ def test_create_service(sample_user): assert service_db.branding == BRANDING_GOVUK assert service_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT assert service_db.research_mode is False + assert service_db.prefix_sms is True assert service.active is True assert sample_user in service_db.users diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 9831091d1..f8dfc7863 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -311,7 +311,7 @@ def test_should_send_sms_sender_from_service_if_present( mmg_client.send_sms.assert_called_once_with( to=validate_and_format_phone_number("+447234123123"), - content="Dear Sir/Madam, Hello. Yours Truly, The Government.", + content="Sample service: Dear Sir/Madam, Hello. Yours Truly, The Government.", reference=str(db_notification.id), sender=service.sms_sender ) @@ -692,23 +692,26 @@ 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'), +@pytest.mark.parametrize('sms_sender, expected_sender, prefix_sms, expected_content', [ + ('foo', 'foo', False, 'bar'), + ('foo', 'foo', True, 'Sample service: bar'), # if 40604 is actually in DB then treat that as if entered manually - ('40604', '40604', 'bar'), + ('40604', '40604', False, 'bar'), # 'testing' is the FROM_NUMBER during unit tests - ('testing', 'testing', 'Sample service: bar'), + ('testing', 'testing', True, 'Sample service: bar'), + ('testing', 'testing', False, 'bar'), ]) def test_should_handle_sms_sender_and_prefix_message( mocker, sms_sender, + prefix_sms, expected_sender, expected_content, notify_db_session ): mocker.patch('app.mmg_client.send_sms') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - service = create_service(sms_sender=sms_sender) + service = create_service(sms_sender=sms_sender, prefix_sms=prefix_sms) template = create_template(service, content='bar') notification = create_notification(template) @@ -725,7 +728,7 @@ def test_should_handle_sms_sender_and_prefix_message( @pytest.mark.parametrize('sms_sender, prefix_setting, expected_content', [ ('foo', True, 'Sample service: bar'), ('foo', False, 'bar'), - ('foo', None, 'bar'), + ('foo', None, 'Sample service: bar'), # 'testing' is the default SMS sender in unit tests ('testing', None, 'Sample service: bar'), ('testing', False, 'bar'), diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 32def58f6..b51a3826b 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1562,18 +1562,18 @@ def test_set_sms_sender_for_service_rejects_null(client, sample_service): assert result['message'] == {'sms_sender': ['Field may not be null.']} -@pytest.mark.parametrize('default_sms_sender, should_prefix', [ - (None, True), # None means use default - ('Foo', False), +@pytest.mark.parametrize('service_attribute, should_prefix', [ + (True, True), + (False, False), ]) def test_prefixing_messages_based_on_sms_sender( client, notify_db_session, - default_sms_sender, + service_attribute, should_prefix, ): service = create_service( - sms_sender=default_sms_sender or current_app.config['FROM_NUMBER'] + prefix_sms=service_attribute ) create_service_sms_sender( service=service,