diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 808cd2cdb..c253748ad 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -73,7 +73,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.get_inbound_number() + sender=service.get_default_sms_sender() ) except Exception as e: dao_toggle_sms_provider(provider.name) diff --git a/app/models.py b/app/models.py index dfe6cabe7..645b5490f 100644 --- a/app/models.py +++ b/app/models.py @@ -275,8 +275,6 @@ class Service(db.Model, Versioned): def get_inbound_number(self): if self.inbound_number and self.inbound_number.active: return self.inbound_number.number - else: - return self.get_default_sms_sender() def get_default_sms_sender(self): default_sms_sender = [x for x in self.service_sms_senders if x.is_default] diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 32b96cf8f..b4c837306 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -95,7 +95,7 @@ def post_notification(notification_type): if notification_type == SMS_TYPE: create_resp_partial = functools.partial( create_post_sms_response_from_notification, - from_number=authenticated_service.get_inbound_number() + from_number=authenticated_service.get_default_sms_sender() ) 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 37b5f2ffb..07b17eb0b 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -12,6 +12,7 @@ import app 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.dao.service_sms_sender_dao import dao_add_sms_sender_for_service from app.delivery import send_to_providers from app.models import ( Notification, @@ -31,8 +32,8 @@ from tests.app.db import ( create_notification, create_inbound_number, create_reply_to_email, - create_reply_to_email_for_notification -) + create_reply_to_email_for_notification, + create_service_sms_sender) def test_should_return_highest_priority_active_provider(restore_provider_details): @@ -699,14 +700,18 @@ def test_should_handle_sms_sender_and_prefix_message( ) -def test_should_use_inbound_number_as_sender_if_set( - sample_service, - mocker +def test_should_use_inbound_number_as_sender_if_default_sms_sender( + notify_db_session, + mocker ): - sample_service.sms_sender = 'test sender' - template = create_template(sample_service, content='bar') + service = create_service(sms_sender='test sender') + inbound_number = create_inbound_number('1') + dao_add_sms_sender_for_service(service_id=service.id, + sms_sender=inbound_number.number, + is_default=True, + inbound_number_id=inbound_number.id) + template = create_template(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') @@ -721,6 +726,32 @@ def test_should_use_inbound_number_as_sender_if_set( ) +def test_should_use_default_sms_sender( + notify_db_session, + mocker +): + service = create_service(sms_sender='test sender') + inbound_number = create_inbound_number('1') + dao_add_sms_sender_for_service(service_id=service.id, + sms_sender=inbound_number.number, + is_default=False, + inbound_number_id=inbound_number.id) + template = create_template(service, content='bar') + notification = create_notification(template) + + 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='test sender' + ) + + def test_send_email_to_provider_get_linked_email_reply_to_default_is_false( sample_service, sample_email_template, diff --git a/tests/app/test_model.py b/tests/app/test_model.py index a1bc1e343..5ae310a19 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -252,17 +252,11 @@ def test_inbound_number_returns_inbound_number(client, notify_db_session): 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): +def test_inbound_number_returns_none_when_no_inbound_number(client, notify_db_session): with set_config(client.application, 'FROM_NUMBER', 'test'): service = create_service(sms_sender=None) - assert service.get_inbound_number() == 'test' + assert not service.get_inbound_number() def test_service_get_default_reply_to_email_address(sample_service): diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 94fea27e5..9d63c6bfb 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -8,7 +8,6 @@ from app.models import ( ScheduledNotification, SCHEDULE_NOTIFICATIONS, EMAIL_TYPE, - INTERNATIONAL_SMS_TYPE, SMS_TYPE ) from flask import json, current_app @@ -25,7 +24,11 @@ from tests.app.conftest import ( sample_template_without_sms_permission ) -from tests.app.db import create_inbound_number, create_service, create_template, create_reply_to_email +from tests.app.db import ( + create_service, + create_template, + create_reply_to_email +) @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -64,15 +67,16 @@ 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): +def test_post_sms_notification_uses_inbound_number_as_sender(client, notify_db_session, mocker): + service = create_service(sms_sender='1', do_create_inbound_number=True) + template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon") mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'phone_number': '+447700900855', - 'template_id': str(sample_template_with_placeholders.id), + 'template_id': str(template.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) + auth_header = create_authorization_header(service_id=service.id) response = client.post( path='/v2/notifications/sms', @@ -85,7 +89,8 @@ def test_post_sms_notification_uses_inbound_number_as_sender(client, sample_temp 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 + assert resp_json['content']['from_number'] == '1' + mocked.assert_called_once_with([str(notification_id)], queue='send-sms-tasks') @pytest.mark.parametrize("notification_type, key_send_to, send_to",