Refactor code to use inbound_numbers if set

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)

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"

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,

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(

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

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

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'

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")])