diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ecce6fb1c..c796a2960 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -175,20 +175,19 @@ def process_job(job_id): ) if template.template_type == 'email': - if service.reply_to_email_address: - from_email = service.reply_to_email_address - else: - from_email = '"{}" <{}@{}>'.format( - service.name, - service.email_from, - current_app.config['NOTIFY_EMAIL_DOMAIN'] - ) + from_email = '"{}" <{}@{}>'.format( + service.name, + service.email_from, + current_app.config['NOTIFY_EMAIL_DOMAIN'] + ) + send_email.apply_async(( str(job.service_id), create_uuid(), from_email.encode('ascii', 'ignore').decode('ascii'), encrypted, datetime.utcnow().strftime(DATETIME_FORMAT)), + {'reply_to_addresses': service.reply_to_email_address}, queue='bulk-email') finished = datetime.utcnow() @@ -284,7 +283,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at): @notify_celery.task(name="send-email") -def send_email(service_id, notification_id, from_address, encrypted_notification, created_at): +def send_email(service_id, notification_id, from_address, encrypted_notification, created_at, reply_to_addresses=None): task_start = monotonic() notification = encryption.decrypt(encrypted_notification) service = dao_fetch_service_by_id(service_id) @@ -329,12 +328,14 @@ def send_email(service_id, notification_id, from_address, encrypted_notification dao_get_template_by_id(notification['template'], notification['template_version']).__dict__, values=notification.get('personalisation', {}) ) + reference = provider.send_email( from_address, notification['to'], template.replaced_subject, body=template.replaced_govuk_escaped, html_body=template.as_HTML_email, + reply_to_addresses=reply_to_addresses, ) update_notification_reference_by_id(notification_id, reference) diff --git a/requirements.txt b/requirements.txt index 723d168df..3e55a74b1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,4 +22,4 @@ statsd==3.2.1 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0 -git+https://github.com/alphagov/notifications-utils.git@5.2.3#egg=notifications-utils==5.2.3 +git+https://github.com/alphagov/notifications-utils.git@5.2.6#egg=notifications-utils==5.2.6 diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 8453e0a85..481976b88 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -250,6 +250,7 @@ def test_should_process_email_job_if_exactly_on_send_limits(notify_db, "something_encrypted", "2016-01-01T11:09:00.061258" ), + {'reply_to_addresses': None}, queue="bulk-email" ) mock_celery_remove_job.assert_called_once_with((str(job.id),), queue="remove-job") @@ -299,6 +300,7 @@ def test_should_process_email_job(sample_email_job, mocker, mock_celery_remove_j "something_encrypted", "2016-01-01T11:09:00.061258" ), + {'reply_to_addresses': None}, queue="bulk-email" ) job = jobs_dao.dao_get_job_by_id(sample_email_job.id) @@ -527,7 +529,8 @@ def test_should_send_email_if_restricted_service_and_valid_email(notify_db, noti "test@restricted.com", template.subject, body=template.content, - html_body=AnyStringWith(template.content) + html_body=AnyStringWith(template.content), + reply_to_addresses=None ) @@ -618,7 +621,8 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh "my_email@my_email.com", notification['personalisation']['name'], body="Hello Jo", - html_body=AnyStringWith("Hello Jo") + html_body=AnyStringWith("Hello Jo"), + reply_to_addresses=None ) statsd_client.incr.assert_called_once_with("notifications.tasks.send-email") @@ -669,7 +673,8 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email "my_email@my_email.com", sample_email_template.subject, body="This is a template", - html_body=AnyStringWith("This is a template") + html_body=AnyStringWith("This is a template"), + reply_to_addresses=None ) persisted_notification = notifications_dao.get_notification(sample_email_template.service_id, notification_id) @@ -704,7 +709,8 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi "my_email@my_email.com", notification['personalisation']['name'], body="Hello Jo", - html_body=AnyStringWith("Hello Jo") + html_body=AnyStringWith("Hello Jo"), + reply_to_addresses=None ) persisted_notification = notifications_dao.get_notification( sample_email_template_with_placeholders.service_id, notification_id @@ -758,7 +764,8 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em "my_email@my_email.com", sample_email_template.subject, body="This is a template", - html_body=AnyStringWith("This is a template") + html_body=AnyStringWith("This is a template"), + reply_to_addresses=None ) @@ -815,7 +822,8 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai "my_email@my_email.com", sample_email_template.subject, body=sample_email_template.content, - html_body=AnyStringWith(sample_email_template.content) + html_body=AnyStringWith(sample_email_template.content), + reply_to_addresses=None ) persisted_notification = notifications_dao.get_notification(sample_email_template.service_id, notification_id) assert persisted_notification.id == notification_id @@ -948,33 +956,7 @@ def test_email_reset_password_should_send_email(notify_db, notify_db_session, no message) -def test_process_email_job_should_use_email_from_if_no_reply_to(sample_email_job, mocker, mock_celery_remove_job): - mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('email')) - mocker.patch('app.celery.tasks.send_email.apply_async') - mocker.patch('app.encryption.encrypt', return_value='something_encrypted') - mocker.patch('app.celery.tasks.create_uuid', return_value='uuid') - - sample_email_job.service.reply_to_email_address = None - - process_job(sample_email_job.id) - - tasks.send_email.apply_async.assert_called_once_with( - ( - str(sample_email_job.service_id), - 'uuid', - "\"{}\" <{}@{}>".format( - sample_email_job.service.name, - sample_email_job.service.email_from, - current_app.config['NOTIFY_EMAIL_DOMAIN'] - ), - 'something_encrypted', - ANY - ), - queue="bulk-email" - ) - - -def test_process_email_job_should_use_reply_to_email(sample_email_job, mocker, mock_celery_remove_job): +def test_process_email_job_should_use_reply_to_email_if_present(sample_email_job, mocker, mock_celery_remove_job): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('email')) mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.encryption.encrypt', return_value='something_encrypted') @@ -987,11 +969,16 @@ def test_process_email_job_should_use_reply_to_email(sample_email_job, mocker, m tasks.send_email.apply_async.assert_called_once_with( ( str(sample_email_job.service_id), - 'uuid', - 'somereply@testservice.gov.uk', - 'something_encrypted', + "uuid", + "\"{}\" <{}@{}>".format( + sample_email_job.service.name, + sample_email_job.service.email_from, + "test.notify.com" + ), + "something_encrypted", ANY ), + {'reply_to_addresses': 'somereply@testservice.gov.uk'}, queue="bulk-email" )