diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 324c21582..ab56a7e05 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -47,11 +47,14 @@ def send_sms_to_provider(notification): "Starting sending SMS {} to provider at {}".format(notification.id, datetime.utcnow()) ) template_model = dao_get_template_by_id(notification.template_id, notification.template_version) + + sender_has_been_customised = (not service.get_prefix_sms_with_service_name()) + template = SMSMessageTemplate( template_model.__dict__, values=notification.personalisation, prefix=service.name, - sender=service.sms_sender not in {None, current_app.config['FROM_NUMBER']} + sender=sender_has_been_customised, ) if service.research_mode or notification.key_type == KEY_TYPE_TEST: diff --git a/app/models.py b/app/models.py index 76bf0edfd..6037bd26d 100644 --- a/app/models.py +++ b/app/models.py @@ -224,6 +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) 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') @@ -280,6 +281,11 @@ class Service(db.Model, Versioned): default_letter_contact = [x for x in self.letter_contacts if x.is_default] return default_letter_contact[0].contact_block if default_letter_contact else None + def get_prefix_sms_with_service_name(self): + if self.prefix_sms is not None: + return self.prefix_sms + return self.get_default_sms_sender() == current_app.config['FROM_NUMBER'] + class AnnualBilling(db.Model): __tablename__ = "annual_billing" diff --git a/app/schemas.py b/app/schemas.py index a143e3bbb..76d91b23d 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -188,6 +188,7 @@ class ServiceSchema(BaseSchema): reply_to_email_address = fields.Method(method_name="get_reply_to_email_address") sms_sender = fields.Method(method_name="get_sms_sender") letter_contact_block = fields.Method(method_name="get_letter_contact") + prefix_sms_with_service_name = fields.Method(method_name="get_prefix_sms_with_service_name") def service_permissions(self, service): return [p.permission for p in service.permissions] @@ -201,6 +202,9 @@ class ServiceSchema(BaseSchema): def get_letter_contact(self, service): return service.get_default_letter_contact() + def get_prefix_sms_with_service_name(self, service): + return service.get_prefix_sms_with_service_name() + class Meta: model = models.Service dump_only = ['reply_to_email_address', 'letter_contact_block'] diff --git a/migrations/versions/0132_add_sms_prefix_setting.py b/migrations/versions/0132_add_sms_prefix_setting.py new file mode 100644 index 000000000..e461a008c --- /dev/null +++ b/migrations/versions/0132_add_sms_prefix_setting.py @@ -0,0 +1,23 @@ +""" + +Revision ID: 0132_add_sms_prefix_setting +Revises: 0131_user_auth_types +Create Date: 2017-11-03 11:07:40.537006 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0132_add_sms_prefix_setting' +down_revision = '0131_user_auth_types' + + +def upgrade(): + op.add_column('services', sa.Column('prefix_sms', sa.Boolean(), nullable=True)) + op.add_column('services_history', sa.Column('prefix_sms', sa.Boolean(), nullable=True)) + + +def downgrade(): + op.drop_column('services_history', 'prefix_sms') + op.drop_column('services', 'prefix_sms') diff --git a/tests/app/db.py b/tests/app/db.py index da401b209..c7c950544 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -68,7 +68,8 @@ def create_service( research_mode=False, active=True, do_create_inbound_number=True, - email_from=None + email_from=None, + prefix_sms=None, ): service = Service( name=service_name, @@ -77,6 +78,7 @@ def create_service( email_from=email_from if email_from else service_name.lower().replace(' ', '.'), created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())), sms_sender=sms_sender, + prefix_sms=prefix_sms, ) dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 2d6a10cdf..9831091d1 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -722,6 +722,39 @@ 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'), + # 'testing' is the default SMS sender in unit tests + ('testing', None, 'Sample service: bar'), + ('testing', False, 'bar'), +]) +def test_should_handle_sms_prefix_setting( + mocker, + sms_sender, + prefix_setting, + 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, + prefix_sms=prefix_setting, + ) + template = create_template(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=ANY, + to=ANY, + reference=ANY, + ) + + def test_should_use_inbound_number_as_sender_if_default_sms_sender( notify_db_session, mocker diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index b281e133b..32def58f6 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -156,6 +156,7 @@ def test_get_service_by_id(client, sample_service): assert json_resp['data']['branding'] == 'govuk' assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] + assert json_resp['data']['prefix_sms_with_service_name'] is True def test_get_service_by_id_returns_free_sms_limit(client, sample_service): @@ -1561,6 +1562,36 @@ 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), +]) +def test_prefixing_messages_based_on_sms_sender( + client, + notify_db_session, + default_sms_sender, + should_prefix, +): + service = create_service( + sms_sender=default_sms_sender or current_app.config['FROM_NUMBER'] + ) + create_service_sms_sender( + service=service, + sms_sender='ignored', + is_default=False, + ) + + result = client.get( + url_for( + 'service.get_service_by_id', + service_id=service.id + ), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + service = json.loads(result.get_data(as_text=True))['data'] + assert service['prefix_sms_with_service_name'] == should_prefix + + @pytest.mark.parametrize('today_only,stats', [ ('False', {'requested': 2, 'delivered': 1, 'failed': 0}), ('True', {'requested': 1, 'delivered': 0, 'failed': 0})