From c36423aac610122bfe066595a5c8367aba46eb7b Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 16 Aug 2017 12:27:42 +0100 Subject: [PATCH] Refactor code for `dao_fetch_servies_by_sms_sender` to use inbound_numbers This will need to be refactored after the deployment of api and admin and after the update script for existing services using inbound numbers has been executed. --- app/dao/services_dao.py | 18 ++++- tests/app/conftest.py | 11 ++- tests/app/dao/test_services_dao.py | 67 +++++++++++++++++-- tests/app/db.py | 12 +++- .../test_receive_notification.py | 27 +++++--- 5 files changed, 113 insertions(+), 22 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 20099c009..b4cd2e649 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,7 +1,7 @@ import uuid from datetime import date, datetime, timedelta -from sqlalchemy import asc, func +from sqlalchemy import asc, func, or_, and_ from sqlalchemy.orm import joinedload from flask import current_app @@ -19,6 +19,7 @@ from app.models import ( Template, TemplateHistory, TemplateRedacted, + InboundNumber, Job, NotificationHistory, Notification, @@ -65,9 +66,22 @@ def dao_fetch_service_by_id(service_id, only_active=False): return query.one() +############ +# refactor this when API only uses inbound_numbers and not sms_sender +############ def dao_fetch_services_by_sms_sender(sms_sender): + inbound_number = InboundNumber.query.filter( + InboundNumber.number == sms_sender + ).first() + + if not inbound_number: + return [] + return Service.query.filter( - Service.sms_sender == sms_sender + or_( + Service.sms_sender == sms_sender, + Service.id == inbound_number.service_id + ) ).all() diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 5b38fa739..74ea2bbd7 100644 --- a/tests/app/conftest.py +++ b/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,12 +157,18 @@ 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 @pytest.fixture(scope='function') def sample_service_full_permissions(notify_db, notify_db_session): - return sample_service(notify_db, notify_db_session, permissions=SERVICE_PERMISSION_TYPES) + service = sample_service(notify_db, notify_db_session, permissions=SERVICE_PERMISSION_TYPES) + create_inbound_number('12345', service_id=service.id) + return service @pytest.fixture(scope='function') diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index b60767289..35fa803ca 100644 --- a/tests/app/dao/test_services_dao.py +++ b/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,67 @@ 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_allocating_inbound_number_shows_on_service(notify_db_session, sample_inbound_numbers): diff --git a/tests/app/db.py b/tests/app/db.py index 676487135..9f7539861 100644 --- a/tests/app/db.py +++ b/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 diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index 0b99d7cbb..8dab45320 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -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" } @@ -123,10 +123,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 +135,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' @@ -147,11 +146,11 @@ def test_create_inbound_mmg_sms_object(sample_service_full_permissions): 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) + inbound_number = sample_service_full_permissions.get_inbound_number() 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', @@ -167,13 +166,23 @@ def test_create_inbound_mmg_sms_object_uses_inbound_number_if_set(sample_service ) assert inbound_sms.service_id == sample_service_full_permissions.id - assert inbound_sms.notify_number == inbound_number.number + 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',