diff --git a/app/dao/provider_details_dao.py b/app/dao/provider_details_dao.py index ff4db1a93..1e2c11727 100644 --- a/app/dao/provider_details_dao.py +++ b/app/dao/provider_details_dao.py @@ -60,6 +60,7 @@ def dao_toggle_sms_provider(identifier): @transactional def dao_switch_sms_provider_to_provider_with_identifier(identifier): new_provider = get_provider_details_by_identifier(identifier) + if provider_is_inactive(new_provider): return @@ -69,7 +70,7 @@ def dao_switch_sms_provider_to_provider_with_identifier(identifier): providers_to_update = [] if conflicting_provider: - providers_to_update = switch_providers(conflicting_provider, new_provider) + switch_providers(conflicting_provider, new_provider) else: current_provider = get_current_provider('sms') if not provider_is_primary(current_provider, new_provider, identifier): @@ -79,10 +80,14 @@ def dao_switch_sms_provider_to_provider_with_identifier(identifier): dao_update_provider_details(provider) -def get_provider_details_by_notification_type(notification_type): - return ProviderDetails.query.filter_by( - notification_type=notification_type - ).order_by(asc(ProviderDetails.priority)).all() +def get_provider_details_by_notification_type(notification_type, supports_international=False): + + filters = [ProviderDetails.notification_type == notification_type] + + if supports_international: + filters.append(ProviderDetails.supports_international == supports_international) + + return ProviderDetails.query.filter(*filters).order_by(asc(ProviderDetails.priority)).all() @transactional diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index f6991c0b0..6634daa32 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -25,7 +25,7 @@ def send_sms_to_provider(notification): return if notification.status == 'created': - provider = provider_to_use(SMS_TYPE, notification.id) + provider = provider_to_use(SMS_TYPE, notification.id, notification.international) current_app.logger.info( "Starting sending SMS {} to provider at {}".format(notification.id, datetime.utcnow()) ) @@ -120,9 +120,9 @@ def update_notification(notification, provider): dao_update_notification(notification) -def provider_to_use(notification_type, notification_id): +def provider_to_use(notification_type, notification_id, international=False): active_providers_in_order = [ - provider for provider in get_provider_details_by_notification_type(notification_type) if provider.active + p for p in get_provider_details_by_notification_type(notification_type, international) if p.active ] if not active_providers_in_order: @@ -147,11 +147,8 @@ def get_logo_url(base_url, branding_path, logo_file): base_url = parse.urlparse(base_url) netloc = base_url.netloc - if ( - base_url.netloc.startswith('localhost') or - # covers both preview and staging - 'notify.works' in base_url.netloc - ): + # covers both preview and staging + if base_url.netloc.startswith('localhost') or 'notify.works' in base_url.netloc: path = '/static' + branding_path + logo_file else: if base_url.netloc.startswith('www'): diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index 5092bd1b9..ed3f0e1f0 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -35,16 +35,23 @@ def test_can_get_all_providers(restore_provider_details): assert len(get_provider_details()) == 5 -def test_can_get_sms_providers(restore_provider_details): +def test_can_get_sms_non_international_providers(restore_provider_details): sms_providers = get_provider_details_by_notification_type('sms') assert len(sms_providers) == 3 assert all('sms' == prov.notification_type for prov in sms_providers) -def test_can_get_sms_providers_in_order_of_priority(restore_provider_details): - providers = get_provider_details_by_notification_type('sms') +def test_can_get_sms_international_providers(restore_provider_details): + sms_providers = get_provider_details_by_notification_type('sms', True) + assert len(sms_providers) == 1 + assert all('sms' == prov.notification_type for prov in sms_providers) + assert all(prov.supports_international for prov in sms_providers) - assert providers[0].priority < providers[1].priority < providers[2].priority + +def test_can_get_sms_providers_in_order_of_priority(restore_provider_details): + providers = get_provider_details_by_notification_type('sms', False) + + assert providers[0].priority < providers[1].priority def test_can_get_email_providers_in_order_of_priority(restore_provider_details): diff --git a/tests/app/db.py b/tests/app/db.py index 195a77ea7..38d5eca9d 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -72,7 +72,8 @@ def create_notification( api_key_id=None, key_type=KEY_TYPE_NORMAL, sent_by=None, - client_reference=None + client_reference=None, + international=False ): if created_at is None: created_at = datetime.utcnow() @@ -103,7 +104,8 @@ def create_notification( 'sent_by': sent_by, 'updated_at': updated_at, 'client_reference': client_reference, - 'job_row_number': job_row_number + 'job_row_number': job_row_number, + 'international': international } notification = Notification(**data) dao_create_notification(notification) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 80b08de4a..9852721c9 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -7,8 +7,9 @@ import pytest from notifications_utils.recipients import validate_and_format_phone_number import app -from app import mmg_client +from app import mmg_client, firetext_client from app.dao import (provider_details_dao, notifications_dao) +from app.dao.provider_details_dao import dao_switch_sms_provider_to_provider_with_identifier from app.delivery import send_to_providers from app.models import ( Notification, @@ -18,7 +19,7 @@ from app.models import ( KEY_TYPE_TEAM, BRANDING_ORG, BRANDING_BOTH, -) + ProviderDetails) from tests.app.db import create_service, create_template, create_notification @@ -471,3 +472,61 @@ def test_should_update_billable_units_according_to_research_mode_and_key_type(no ) assert sample_notification.billable_units == billable_units + + +def test_should_send_sms_to_international_providers( + restore_provider_details, + sample_sms_template_with_html, + sample_user, + mocker +): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) + + dao_switch_sms_provider_to_provider_with_identifier('firetext') + + db_notification_uk = create_notification( + template=sample_sms_template_with_html, + to_field="+447234123999", + personalisation={"name": "Jo"}, + status='created', + international=False) + + db_notification_international = create_notification( + template=sample_sms_template_with_html, + to_field="+447234123111", + personalisation={"name": "Jo"}, + status='created', + international=True) + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.firetext_client.send_sms') + + send_to_providers.send_sms_to_provider( + db_notification_uk + ) + + firetext_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123999")), + content=ANY, + reference=str(db_notification_uk.id), + sender=None + ) + + send_to_providers.send_sms_to_provider( + db_notification_international + ) + + mmg_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123111")), + content=ANY, + reference=str(db_notification_international.id), + sender=None + ) + + notification_uk = Notification.query.filter_by(id=db_notification_uk.id).one() + notification_int = Notification.query.filter_by(id=db_notification_international.id).one() + + assert notification_uk.status == 'sending' + assert notification_uk.sent_by == 'firetext' + assert notification_int.status == 'sending' + assert notification_int.sent_by == 'mmg'