From 667ee57a35a20811658870db6965ac1c1140976f Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 14 Aug 2017 19:47:09 +0100 Subject: [PATCH] Refactor code to use inbound_numbers if set --- app/delivery/send_to_providers.py | 2 +- app/models.py | 6 +++++ app/notifications/receive_notifications.py | 2 +- app/v2/notifications/post_notifications.py | 3 +-- tests/app/delivery/test_send_to_providers.py | 24 ++++++++++++++++- .../test_receive_notification.py | 27 ++++++++++++++++++- tests/app/test_model.py | 21 +++++++++++++++ .../notifications/test_post_notifications.py | 25 ++++++++++++++++- 8 files changed, 103 insertions(+), 7 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index fcf4348d8..c9aa84a20 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -51,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 or current_app.config['FROM_NUMBER'] + sender=service.get_inbound_number() ) except Exception as e: dao_toggle_sms_provider(provider.name) diff --git a/app/models.py b/app/models.py index 9cd0b12b6..e0a0ad474 100644 --- a/app/models.py +++ b/app/models.py @@ -241,6 +241,12 @@ class Service(db.Model, Versioned): return cls(**fields) + def get_inbound_number(self): + if self.inbound_number and self.inbound_number.active: + return self.inbound_number.number + else: + return self.sms_sender or current_app.config['FROM_NUMBER'] + class InboundNumber(db.Model): __tablename__ = "inbound_numbers" diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index 34e726cf1..4fd3d33c2 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -107,7 +107,7 @@ def create_inbound_sms_object(service, content, from_number, provider_ref, date_ inbound = InboundSms( service=service, - notify_number=service.sms_sender, + notify_number=service.get_inbound_number(), user_number=user_number, provider_date=provider_date, provider_reference=provider_ref, diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index d18bb3b4d..c27ca8dc5 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -80,10 +80,9 @@ def post_notification(notification_type): ) if notification_type == SMS_TYPE: - sms_sender = authenticated_service.sms_sender or current_app.config.get('FROM_NUMBER') create_resp_partial = functools.partial( create_post_sms_response_from_notification, - from_number=sms_sender + from_number=authenticated_service.get_inbound_number() ) elif notification_type == EMAIL_TYPE: create_resp_partial = functools.partial( diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index d14fee6d8..35368becf 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -21,7 +21,7 @@ from app.models import ( BRANDING_ORG, BRANDING_BOTH) -from tests.app.db import create_service, create_template, create_notification +from tests.app.db import create_service, create_template, create_notification, create_inbound_number def test_should_return_highest_priority_active_provider(restore_provider_details): @@ -643,3 +643,25 @@ def test_should_handle_sms_sender_and_prefix_message( to=ANY, reference=ANY, ) + + +def test_should_use_inbound_number_as_sender_if_set( + sample_service, + mocker +): + sample_service.sms_sender = 'test sender' + template = create_template(sample_service, content='bar') + notification = create_notification(template) + inbound_number = create_inbound_number('1', service_id=sample_service.id) + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') + + send_to_providers.send_sms_to_provider(notification) + + mmg_client.send_sms.assert_called_once_with( + to=ANY, + content=ANY, + reference=str(notification.id), + sender=inbound_number.number + ) diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index 36392689f..0b99d7cbb 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -15,7 +15,7 @@ from app.notifications.receive_notifications import ( ) from app.models import InboundSms, EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE -from tests.app.db import create_service +from tests.app.db import create_inbound_number, create_service from tests.app.conftest import sample_service @@ -145,6 +145,31 @@ def test_create_inbound_mmg_sms_object(sample_service_full_permissions): assert inbound_sms.provider == 'mmg' +def test_create_inbound_mmg_sms_object_uses_inbound_number_if_set(sample_service_full_permissions): + sample_service_full_permissions.sms_sender = 'foo' + inbound_number = create_inbound_number('1', service_id=sample_service_full_permissions.id) + + data = { + 'Message': 'hello+there+%F0%9F%93%A9', + 'Number': 'foo', + 'MSISDN': '07700 900 001', + 'DateRecieved': '2017-01-02+03%3A04%3A05', + 'ID': 'bar', + } + + inbound_sms = create_inbound_sms_object( + sample_service_full_permissions, + format_mmg_message(data["Message"]), + data["MSISDN"], + data["ID"], + data["DateRecieved"], + "mmg" + ) + + assert inbound_sms.service_id == sample_service_full_permissions.id + assert inbound_sms.notify_number == inbound_number.number + + @pytest.mark.parametrize('notify_number', ['foo', 'baz'], ids=['two_matching_services', 'no_matching_services']) def test_receive_notification_error_if_not_single_matching_service(client, notify_db_session, notify_number): create_service(service_name='a', sms_sender='foo', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE]) diff --git a/tests/app/test_model.py b/tests/app/test_model.py index b8ffcf9b4..56a121e01 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -20,6 +20,7 @@ from tests.app.conftest import ( sample_notification_with_job as create_sample_notification_with_job ) from tests.app.db import create_notification, create_service, create_inbound_number +from tests.conftest import set_config @pytest.mark.parametrize('mobile_number', [ @@ -227,3 +228,23 @@ def test_inbound_number_serializes_with_service(client, notify_db_session): assert serialized_inbound_number.get('id') == str(inbound_number.id) assert serialized_inbound_number.get('service').get('id') == str(inbound_number.service.id) assert serialized_inbound_number.get('service').get('name') == inbound_number.service.name + + +def test_inbound_number_returns_inbound_number(client, notify_db_session): + service = create_service() + inbound_number = create_inbound_number(number='1', service_id=service.id) + + assert service.get_inbound_number() == inbound_number.number + + +def test_inbound_number_returns_sms_sender(client, notify_db_session): + service = create_service(sms_sender='testing') + + assert service.get_inbound_number() == service.sms_sender + + +def test_inbound_number_returns_from_number_config(client, notify_db_session): + with set_config(client.application, 'FROM_NUMBER', 'test'): + service = create_service(sms_sender=None) + + assert service.get_inbound_number() == 'test' diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index c108faacf..bd5b4f711 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -17,7 +17,7 @@ from tests.app.conftest import ( sample_template_without_sms_permission, sample_template_without_email_permission ) -from tests.app.db import create_service, create_template +from tests.app.db import create_inbound_number, create_service, create_template @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -55,6 +55,29 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol assert mocked.called +def test_post_sms_notification_uses_inbound_number_as_sender(client, sample_template_with_placeholders, mocker): + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + data = { + 'phone_number': '+447700900855', + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'} + } + inbound_number = create_inbound_number('1', service_id=sample_template_with_placeholders.service_id) + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + + response = client.post( + path='/v2/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + notifications = Notification.query.all() + assert len(notifications) == 1 + notification_id = notifications[0].id + assert resp_json['id'] == str(notification_id) + assert resp_json['content']['from_number'] == inbound_number.number + + @pytest.mark.parametrize("notification_type, key_send_to, send_to", [("sms", "phone_number", "+447700900855"), ("email", "email_address", "sample@email.com")])