From cccffdd2acb443f1dd267df166abba0d48cc58c7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 3 Nov 2017 11:39:25 +0000 Subject: [PATCH] Add a column for a service to set SMS prefixing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In future changes, services will be able to control whether their text messages will be prefixed with the name of their service. This commit: - adds a column to store the value of that setting - makes the service model take notice of it, if it were to have a value set It doesn’t: - provide a way of setting the value of this column Currently the column can have three values: - `None` – ignore it (this is what all current services will start as) and continue to determine whether to prefix messages by looking at the sender - `True` – always the service name to the start of text messages - `False` – never add the service name to the start of text messages In the future we’ll migrate all services to be either `True` or `False`, the `None` will go away and all services will have direct control over the setting. --- app/delivery/send_to_providers.py | 2 +- app/models.py | 5 ++- app/schemas.py | 2 +- .../versions/0132_add_sms_prefix_setting.py | 23 +++++++++++++ tests/app/db.py | 4 ++- tests/app/delivery/test_send_to_providers.py | 33 +++++++++++++++++++ 6 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 migrations/versions/0132_add_sms_prefix_setting.py diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 74316dc08..ab56a7e05 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -48,7 +48,7 @@ def send_sms_to_provider(notification): ) template_model = dao_get_template_by_id(notification.template_id, notification.template_version) - sender_has_been_customised = (not service.prefix_sms_with_service_name()) + sender_has_been_customised = (not service.get_prefix_sms_with_service_name()) template = SMSMessageTemplate( template_model.__dict__, diff --git a/app/models.py b/app/models.py index 1c571ad2c..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,7 +281,9 @@ 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 prefix_sms_with_service_name(self): + 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'] diff --git a/app/schemas.py b/app/schemas.py index 3916c27bb..76d91b23d 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -203,7 +203,7 @@ class ServiceSchema(BaseSchema): return service.get_default_letter_contact() def get_prefix_sms_with_service_name(self, service): - return service.prefix_sms_with_service_name() + return service.get_prefix_sms_with_service_name() class Meta: model = models.Service 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 4be3a3799..c070e4dca 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -67,7 +67,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, @@ -76,6 +77,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