Merge pull request #1178 from alphagov/ken-use-inbound-number

Use inbound number for delivering sms and to show on service settings

tests/app/conftest.py

@@ -25,9 +25,10 @@ from app.models import (
     ProviderDetailsHistory,
     ProviderRates,
     NotificationStatistics,
+    ScheduledNotification,
     ServiceWhitelist,
     KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM,
-    MOBILE_TYPE, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED, ScheduledNotification,
+    MOBILE_TYPE, EMAIL_TYPE, INBOUND_SMS_TYPE, SMS_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED,
     SERVICE_PERMISSION_TYPES)
 from app.dao.users_dao import (create_user_code, create_secret_code)
 from app.dao.services_dao import (dao_create_service, dao_add_user_to_service)
@@ -156,6 +157,10 @@ def sample_service(
     else:
         if user not in service.users:
             dao_add_user_to_service(service, user)
+
+    if INBOUND_SMS_TYPE in permissions:
+        create_inbound_number('12345', service_id=service.id)
+
     return service
 
 
@@ -1051,8 +1056,11 @@ def admin_request(client):
                 data=json.dumps(_data),
                 headers=[('Content-Type', 'application/json'), create_authorization_header()]
             )
-            json_resp = json.loads(resp.get_data(as_text=True))
-            assert resp.status_code == _expected_status, json_resp
+            if resp.get_data():
+                json_resp = json.loads(resp.get_data(as_text=True))
+            else:
+                json_resp = None
+            assert resp.status_code == _expected_status
             return json_resp
 
         @staticmethod

tests/app/dao/test_inbound_numbers_dao.py

@@ -74,7 +74,7 @@ def test_set_inbound_number_active_flag(notify_db, notify_db_session, sample_ser
     inbound_number = create_inbound_number(number='1')
     dao_set_inbound_number_to_service(sample_service.id, inbound_number)
 
-    dao_set_inbound_number_active_flag(inbound_number.id, active=active)
+    dao_set_inbound_number_active_flag(sample_service.id, active=active)
 
     inbound_number = dao_get_inbound_number_for_service(sample_service.id)
 

tests/app/dao/test_services_dao.py

@@ -61,7 +61,7 @@ from app.models import (
     SERVICE_PERMISSION_TYPES
 )
 
-from tests.app.db import create_user, create_service
+from tests.app.db import create_inbound_number, create_user, create_service
 from tests.app.conftest import (
     sample_notification as create_notification,
     sample_notification_history as create_notification_history,
@@ -892,14 +892,76 @@ def test_dao_fetch_active_users_for_service_returns_active_only(notify_db, notif
     assert len(users) == 1
 
 
-def test_dao_fetch_services_by_sms_sender(notify_db_session):
-    foo1 = create_service(service_name='a', sms_sender='foo')
-    foo2 = create_service(service_name='b', sms_sender='foo')
-    bar = create_service(service_name='c', sms_sender='bar')
+def test_dao_fetch_services_by_sms_sender_with_inbound_number(notify_db_session):
+    foo1 = create_service(service_name='a', sms_sender='1')
+    foo2 = create_service(service_name='b', sms_sender='2')
+    bar = create_service(service_name='c', sms_sender='3')
+    create_inbound_number('1')
+    create_inbound_number('2')
+    create_inbound_number('3')
 
-    services = dao_fetch_services_by_sms_sender('foo')
+    services = dao_fetch_services_by_sms_sender('1')
 
-    assert {foo1.id, foo2.id} == {x.id for x in services}
+    assert len(services) == 1
+    assert foo1.id == services[0].id
+
+
+def test_dao_fetch_services_by_sms_sender_with_inbound_number_not_set(notify_db_session):
+    create_inbound_number('1')
+
+    services = dao_fetch_services_by_sms_sender('1')
+
+    assert services == []
+
+
+def test_dao_fetch_services_by_sms_sender_when_inbound_number_set(notify_db_session):
+    service = create_service(service_name='a', sms_sender=None)
+    service = create_service(service_name='b')
+    inbound_number = create_inbound_number('1', service_id=service.id)
+
+    services = dao_fetch_services_by_sms_sender('1')
+
+    assert len(services) == 1
+    assert services[0].id == service.id
+
+
+def test_dao_fetch_services_by_sms_sender_when_inbound_number_set_and_sms_sender_same(notify_db_session):
+    service = create_service(service_name='a', sms_sender=None)
+    service = create_service(service_name='b', sms_sender='1')
+    inbound_number = create_inbound_number('1', service_id=service.id)
+
+    services = dao_fetch_services_by_sms_sender('1')
+
+    assert len(services) == 1
+    assert services[0].id == service.id
+
+
+def test_dao_fetch_services_by_sms_sender_when_inbound_number_not_set_gets_sms_sender(notify_db_session):
+    service = create_service(service_name='a', sms_sender=None)
+    service = create_service(service_name='b', sms_sender='testing_gov')
+    inbound_number = create_inbound_number('1', service_id=service.id)
+
+    services = dao_fetch_services_by_sms_sender('testing_gov')
+
+    assert services == []
+
+
+def test_dao_fetch_services_by_sms_sender_with_unknown_number(notify_db_session):
+    service = create_service(service_name='a', sms_sender=None)
+    inbound_number = create_inbound_number('1', service_id=service.id)
+
+    services = dao_fetch_services_by_sms_sender('9')
+
+    assert services == []
+
+
+def test_dao_fetch_services_by_sms_sender_with_inactive_number_returns_empty(notify_db_session):
+    service = create_service(service_name='a', sms_sender=None)
+    inbound_number = create_inbound_number('1', service_id=service.id, active=False)
+
+    services = dao_fetch_services_by_sms_sender('1')
+
+    assert services == []
 
 
 def test_dao_allocating_inbound_number_shows_on_service(notify_db_session, sample_inbound_numbers):

tests/app/db.py

@@ -21,8 +21,11 @@ from app.models import (
     InboundSms,
     InboundNumber,
     Organisation,
-    ServiceInboundApi
-)
+    EMAIL_TYPE,
+    SMS_TYPE,
+    INBOUND_SMS_TYPE,
+    KEY_TYPE_NORMAL,
+    ServiceInboundApi)
 from app.dao.users_dao import save_model_user
 from app.dao.notifications_dao import dao_create_notification, dao_created_scheduled_notification
 from app.dao.templates_dao import dao_create_template
@@ -57,6 +60,7 @@ def create_service(
     sms_sender='testing',
     research_mode=False,
     active=True,
+    do_create_inbound_number=True,
 ):
     service = Service(
         name=service_name,
@@ -66,6 +70,10 @@ def create_service(
         created_by=user or create_user(),
         sms_sender=sms_sender,
     )
+
+    if do_create_inbound_number and INBOUND_SMS_TYPE in service_permissions:
+        create_inbound_number(number=sms_sender, service_id=service.id)
+
     dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions)
 
     service.active = active

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/inbound_number/test_rest.py

@@ -0,0 +1,117 @@
+import pytest
+
+from flask import url_for
+import json
+
+from app.models import InboundNumber
+from app.dao.inbound_numbers_dao import dao_get_inbound_number_for_service
+
+from tests.app.db import create_service, create_inbound_number
+
+
+def test_rest_get_inbound_numbers_when_none_set_returns_empty_list(admin_request):
+    result = admin_request.get('inbound_number.get_inbound_numbers')
+
+    assert result['data'] == []
+
+
+def test_rest_get_inbound_numbers(admin_request, sample_inbound_numbers):
+    result = admin_request.get('inbound_number.get_inbound_numbers')
+
+    assert len(result['data']) == len(sample_inbound_numbers)
+    assert result['data'] == [i.serialize() for i in sample_inbound_numbers]
+
+
+def test_rest_get_inbound_number(admin_request, notify_db_session, sample_service):
+    inbound_number = create_inbound_number(number='1', provider='mmg', active=False, service_id=sample_service.id)
+
+    result = admin_request.get(
+        'inbound_number.get_inbound_number_for_service',
+        service_id=sample_service.id
+    )
+    assert result['data'] == inbound_number.serialize()
+
+
+def test_rest_get_inbound_number_when_service_is_not_assigned_returns_empty_dict(
+        admin_request, notify_db_session, sample_service):
+    result = admin_request.get(
+        'inbound_number.get_inbound_number_for_service',
+        service_id=sample_service.id
+    )
+    assert result['data'] == {}
+
+
+def test_rest_allocate_inbound_number_to_service(
+        admin_request, notify_db_session, sample_service):
+    service = create_service(service_name='test service 1')
+    inbound_number = create_inbound_number(number='1', provider='mmg', active=True)
+
+    result = admin_request.post(
+        'inbound_number.post_allocate_inbound_number',
+        _expected_status=204,
+        service_id=service.id
+    )
+
+    inbound_number_from_db = dao_get_inbound_number_for_service(service.id)
+
+    assert inbound_number_from_db.active
+    assert inbound_number_from_db.id == inbound_number.id
+    assert inbound_number_from_db.number == inbound_number.number
+
+
+def test_rest_allocate_inbound_number_to_service_raises_400_when_no_available_numbers(
+        admin_request, notify_db_session, sample_service):
+    service = create_service(service_name='test service 1')
+    create_inbound_number(number='1', provider='mmg', active=False)
+
+    result = admin_request.post(
+        'inbound_number.post_allocate_inbound_number',
+        _expected_status=400,
+        service_id=service.id
+    )
+
+    assert result['message'] == 'No available inbound numbers'
+
+
+def test_rest_allocate_inbound_number_to_service_sets_active_flag_true_when_flag_is_false(
+        admin_request, notify_db_session, sample_service):
+    service = create_service(service_name='test service 1')
+    create_inbound_number(number='1', provider='mmg', active=False, service_id=service.id)
+
+    result = admin_request.post(
+        'inbound_number.post_allocate_inbound_number',
+        _expected_status=204,
+        service_id=service.id
+    )
+
+    inbound_number = dao_get_inbound_number_for_service(service.id)
+
+    assert inbound_number.active
+
+
+def test_rest_allocate_inbound_number_to_service_sets_active_flag_true_when_flag_is_true(
+        admin_request, notify_db_session, sample_service):
+    service = create_service(service_name='test service 1')
+    create_inbound_number(number='1', provider='mmg', active=True, service_id=service.id)
+
+    result = admin_request.post(
+        'inbound_number.post_allocate_inbound_number',
+        _expected_status=200,
+        service_id=service.id
+    )
+
+
+def test_rest_set_inbound_number_active_flag_off(
+        admin_request, notify_db_session):
+    service = create_service(service_name='test service 1')
+    inbound_number = create_inbound_number(
+        number='1', provider='mmg', active=True, service_id=service.id)
+
+    admin_request.post(
+        'inbound_number.post_set_inbound_number_off',
+        _expected_status=204,
+        service_id=service.id
+    )
+
+    inbound_number_from_db = dao_get_inbound_number_for_service(service.id)
+    assert not inbound_number_from_db.active

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
 
 
@@ -26,7 +26,7 @@ def test_receive_notification_returns_received_to_mmg(client, mocker, sample_ser
         "MSISDN": "447700900855",
         "Message": "Some message to notify",
         "Trigger": "Trigger?",
-        "Number": "testing",
+        "Number": sample_service_full_permissions.get_inbound_number(),
         "Channel": "SMS",
         "DateRecieved": "2012-06-27 12:33:00"
     }
@@ -93,6 +93,36 @@ def test_receive_notification_without_permissions_does_not_create_inbound(
     mocked_send_inbound_sms.assert_not_called()
 
 
+def test_receive_notification_without_permissions_does_not_create_inbound_even_with_inbound_number_set(
+        client, mocker, notify_db, notify_db_session):
+    service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE])
+    inbound_number = create_inbound_number('1', service_id=service.id, active=True)
+
+    mocked_send_inbound_sms = mocker.patch(
+        "app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
+    mocked_has_permissions = mocker.patch(
+        "app.notifications.receive_notifications.has_inbound_sms_permissions", return_value=False)
+
+    data = json.dumps({
+        "ID": "1234",
+        "MSISDN": "447700900855",
+        "Message": "Some message to notify",
+        "Trigger": "Trigger?",
+        "Number": inbound_number.number,
+        "Channel": "SMS",
+        "DateRecieved": "2012-06-27 12:33:00"
+    })
+
+    response = client.post(path='/notifications/sms/receive/mmg',
+                           data=data,
+                           headers=[('Content-Type', 'application/json')])
+
+    assert response.status_code == 200
+    assert len(InboundSms.query.all()) == 0
+    assert mocked_has_permissions.called
+    mocked_send_inbound_sms.assert_not_called()
+
+
 @pytest.mark.parametrize('permissions,expected_response', [
     ([SMS_TYPE, INBOUND_SMS_TYPE], True),
     ([INBOUND_SMS_TYPE], False),
@@ -123,10 +153,9 @@ def test_format_mmg_datetime(provider_date, expected_output):
 
 
 def test_create_inbound_mmg_sms_object(sample_service_full_permissions):
-    sample_service_full_permissions.sms_sender = 'foo'
     data = {
         'Message': 'hello+there+%F0%9F%93%A9',
-        'Number': 'foo',
+        'Number': sample_service_full_permissions.get_inbound_number(),
         'MSISDN': '07700 900 001',
         'DateRecieved': '2017-01-02+03%3A04%3A05',
         'ID': 'bar',
@@ -136,7 +165,7 @@ def test_create_inbound_mmg_sms_object(sample_service_full_permissions):
                                             data["MSISDN"], data["ID"], data["DateRecieved"], "mmg")
 
     assert inbound_sms.service_id == sample_service_full_permissions.id
-    assert inbound_sms.notify_number == 'foo'
+    assert inbound_sms.notify_number == sample_service_full_permissions.get_inbound_number()
     assert inbound_sms.user_number == '447700900001'
     assert inbound_sms.provider_date == datetime(2017, 1, 2, 3, 4, 5)
     assert inbound_sms.provider_reference == 'bar'
@@ -145,10 +174,45 @@ 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 = sample_service_full_permissions.get_inbound_number()
+
+    data = {
+        'Message': 'hello+there+%F0%9F%93%A9',
+        'Number': sample_service_full_permissions.get_inbound_number(),
+        '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
+
+
 @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])
-    create_service(service_name='b', sms_sender='foo', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])
+    create_service(
+        service_name='a',
+        sms_sender='foo',
+        service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE],
+        do_create_inbound_number=False
+    )
+    create_service(
+        service_name='b',
+        sms_sender='foo',
+        service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE],
+        do_create_inbound_number=False
+    )
 
     data = {
         'Message': 'hello',

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