diff --git a/app/celery/tasks.py b/app/celery/tasks.py index f1183b144..a35e2e457 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -402,7 +402,7 @@ def create_dvla_file_contents_for_notifications(notifications): notification.template.__dict__, notification.personalisation, notification_reference=notification.reference, - contact_block=notification.service.get_default_letter_contact(), + contact_block=notification.reply_to_text, org_id=notification.service.dvla_organisation.id, )) for notification in notifications diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 5ec949fec..9ba10c57e 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -70,15 +70,11 @@ def send_sms_to_provider(notification): raise else: try: - sms_sender = dao_get_notification_sms_sender_mapping(notification.id) - if not sms_sender: - sms_sender = service.get_default_sms_sender() - provider.send_sms( to=validate_and_format_phone_number(notification.to, international=notification.international), content=str(template), reference=str(notification.id), - sender=sms_sender + sender=notification.reply_to_text ) except Exception as e: dao_toggle_sms_provider(provider.name) @@ -129,10 +125,7 @@ def send_email_to_provider(notification): from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']) - email_reply_to = dao_get_notification_email_reply_for_notification(notification.id) - - if not email_reply_to: - email_reply_to = service.get_default_reply_to_email_address() + email_reply_to = notification.reply_to_text reference = provider.send_email( from_address, diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 444469731..73b0b376a 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1107,8 +1107,8 @@ def test_create_dvla_file_contents(notify_db_session, mocker): create_letter_contact(service=service, contact_block='London,\nNW1A 1AA') letter_template = create_template(service=service, template_type=LETTER_TYPE) job = create_job(template=letter_template, notification_count=2) - create_notification(template=job.template, job=job, reference=1) - create_notification(template=job.template, job=job, reference=2) + create_notification(template=job.template, job=job, reference=1, reply_to_text=service.get_default_letter_contact()) + create_notification(template=job.template, job=job, reference=2, reply_to_text='Not the default address') mocked_letter_template = mocker.patch("app.celery.tasks.LetterDVLATemplate") mocked_letter_template_instance = mocked_letter_template.return_value mocked_letter_template_instance.__str__.return_value = "dvla|string" @@ -1123,7 +1123,8 @@ def test_create_dvla_file_contents(notify_db_session, mocker): assert not calls[0][0][1] assert not calls[1][0][1] # Named arguments - assert calls[1][1]['contact_block'] == 'London,\nNW1A 1AA' + assert calls[0][1]['contact_block'] == 'London,\nNW1A 1AA' + assert calls[1][1]['contact_block'] == 'Not the default address' assert calls[0][1]['notification_reference'] == '1' assert calls[1][1]['notification_reference'] == '2' assert calls[1][1]['org_id'] == '001' diff --git a/tests/app/db.py b/tests/app/db.py index 1da012e0a..41b6691ca 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -162,7 +162,8 @@ def create_notification( scheduled_for=None, normalised_to=None, one_off=False, - sms_sender_id=None + sms_sender_id=None, + reply_to_text=None ): if created_at is None: created_at = datetime.utcnow() @@ -206,7 +207,8 @@ def create_notification( 'rate_multiplier': rate_multiplier, 'international': international, 'phone_prefix': phone_prefix, - 'normalised_to': normalised_to + 'normalised_to': normalised_to, + 'reply_to_text': reply_to_text } 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 5779a55c3..26bb09c7f 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -1,18 +1,17 @@ import uuid -from datetime import datetime from collections import namedtuple +from datetime import datetime from unittest.mock import ANY, call import pytest -from notifications_utils.recipients import validate_and_format_phone_number from flask import current_app +from notifications_utils.recipients import validate_and_format_phone_number from requests import HTTPError 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, @@ -25,12 +24,10 @@ from app.models import ( BRANDING_BOTH, BRANDING_ORG_BANNER ) - from tests.app.db import ( create_service, create_template, create_notification, - create_inbound_number, create_reply_to_email, create_reply_to_email_for_notification, create_service_sms_sender, @@ -76,7 +73,8 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( ): db_notification = create_notification(template=sample_sms_template_with_html, to_field="+447234123123", personalisation={"name": "Jo"}, - status='created') + status='created', + reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender()) mocker.patch('app.mmg_client.send_sms') stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') @@ -169,7 +167,8 @@ def test_should_not_send_sms_message_when_service_is_inactive_notifcation_is_in_ def test_send_sms_should_use_template_version_from_notification_not_latest( sample_template, mocker): - db_notification = create_notification(template=sample_template, to_field='+447234123123', status='created') + db_notification = create_notification(template=sample_template, to_field='+447234123123', status='created', + reply_to_text=sample_template.service.get_default_sms_sender()) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') @@ -330,7 +329,8 @@ def test_send_sms_should_use_service_sms_sender( mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456', is_default=False) - db_notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id) + db_notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id, + reply_to_text=sms_sender.sms_sender) send_to_providers.send_sms_to_provider( db_notification, @@ -409,7 +409,7 @@ def test_send_email_should_use_service_reply_to_email( mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - db_notification = create_notification(template=sample_email_template) + db_notification = create_notification(template=sample_email_template, reply_to_text='foo@bar.com') create_reply_to_email(service=sample_service, email_address='foo@bar.com') send_to_providers.send_email_to_provider( @@ -422,7 +422,7 @@ def test_send_email_should_use_service_reply_to_email( ANY, body=ANY, html_body=ANY, - reply_to_address=sample_service.get_default_reply_to_email_address() + reply_to_address='foo@bar.com' ) @@ -578,14 +578,18 @@ def test_should_send_sms_to_international_providers( to_field="+447234123999", personalisation={"name": "Jo"}, status='created', - international=False) + international=False, + reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender() + ) db_notification_international = create_notification( template=sample_sms_template_with_html, to_field="+447234123111", personalisation={"name": "Jo"}, status='created', - international=True) + international=True, + reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender() + ) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.firetext_client.send_sms') @@ -692,7 +696,7 @@ def test_should_handle_sms_sender_and_prefix_message( mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') service = create_service_with_defined_sms_sender(sms_sender_value=sms_sender, prefix_sms=prefix_sms) template = create_template(service, content='bar') - notification = create_notification(template) + notification = create_notification(template, reply_to_text=sms_sender) send_to_providers.send_sms_to_provider(notification) @@ -711,7 +715,7 @@ def test_should_use_inbound_number_as_sender_if_default_sms_sender( service = create_service_with_inbound_number(inbound_number='inbound') create_service_sms_sender(service=service, sms_sender="sms_sender", is_default=False) template = create_template(service, content='bar') - notification = create_notification(template) + notification = create_notification(template, reply_to_text='inbound') mocker.patch('app.mmg_client.send_sms') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') @@ -732,7 +736,7 @@ def test_should_use_default_sms_sender( ): service = create_service_with_defined_sms_sender(sms_sender_value="test sender") template = create_template(service, content='bar') - notification = create_notification(template) + notification = create_notification(template, reply_to_text=service.get_default_sms_sender()) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') @@ -754,8 +758,8 @@ def test_send_email_to_provider_get_linked_email_reply_to_default_is_false( mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - db_notification = create_notification(template=sample_email_template) - create_reply_to_email(service=sample_service, email_address='foo@bar.com') + db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com") + create_reply_to_email(service=sample_service, email_address="test@test.com") reply_to = create_reply_to_email_for_notification( db_notification.id, @@ -785,8 +789,10 @@ def test_send_email_to_provider_get_linked_email_reply_to_create_service_email_a mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - db_notification = create_notification(template=sample_email_template) + db_notification = create_notification(template=sample_email_template, + reply_to_text="test@test.com") + # notification_to_email_reply_to is being deprecated. reply_to = create_reply_to_email_for_notification( db_notification.id, sample_service, @@ -816,7 +822,7 @@ def test_send_email_to_provider_should_format_reply_to_email_address( mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - db_notification = create_notification(template=sample_email_template) + db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com\t") reply_to = create_reply_to_email_for_notification( db_notification.id,