From 242be97bfa263da8d54350b65857ec8daa889884 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 4 Jul 2016 17:29:41 +0100 Subject: [PATCH] remove reply_to_addresses from task kwargs also added a test for aws_ses.send_email to prove it handles reply_to_address correctly --- app/celery/provider_tasks.py | 2 +- app/celery/tasks.py | 1 - tests/app/celery/test_provider_tasks.py | 30 ------------------------- tests/app/celery/test_tasks.py | 21 ----------------- tests/app/clients/test_aws_ses.py | 28 +++++++++++++++++++++++ 5 files changed, 29 insertions(+), 53 deletions(-) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 247eaf3a5..c4638a456 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -122,7 +122,7 @@ def provider_to_use(notification_type, notification_id): @notify_celery.task(bind=True, name="send-email-to-provider", max_retries=5, default_retry_delay=5) -def send_email_to_provider(self, service_id, notification_id, reply_to_addresses=None): +def send_email_to_provider(self, service_id, notification_id): task_start = monotonic() service = dao_fetch_service_by_id(service_id) provider = provider_to_use(EMAIL_TYPE, notification_id) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 3bbb3ef37..5d292f7b7 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -167,7 +167,6 @@ def send_email(self, service_id, notification_id, encrypted_notification, created_at, - reply_to_addresses=None, api_key_id=None, key_type=KEY_TYPE_NORMAL): task_start = monotonic() diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 57c8ddbec..018e83758 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -447,33 +447,3 @@ def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_c app.aws_ses_client.send_email.assert_not_called() app.celery.research_mode_tasks.send_email_response.apply_async.assert_not_called() - - -def test_send_email_should_use_service_reply_to_email( - notify_db, notify_db_session, - sample_service, - sample_email_template, - mocker): - mocker.patch('app.statsd_client.incr') - mocker.patch('app.statsd_client.timing') - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') - - db_notification = sample_notification(notify_db, notify_db_session, template=sample_email_template) - sample_service.reply_to_email_address = 'foo@bar.com' - - send_email_to_provider( - db_notification.service_id, - db_notification.id, - reply_to_addresses="waz@baz.com" - ) - - aws_ses_client.send_email.assert_called_once_with( - ANY, - ANY, - ANY, - body=ANY, - html_body=ANY, - reply_to_address=sample_service.reply_to_email_address - ) diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 6cbe2ead8..e5f2550cd 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -647,24 +647,3 @@ def test_send_email_should_go_to_retry_queue_if_database_errors(sample_email_tem with pytest.raises(NoResultFound) as e: Notification.query.filter_by(id=notification_id).one() assert 'No row was found for one' in str(e.value) - - -def test_process_email_should_not_use_reply_to_email(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 = 'somereply@testservice.gov.uk' - - process_job(sample_email_job.id) - - tasks.send_email.apply_async.assert_called_once_with( - ( - str(sample_email_job.service_id), - "uuid", - "something_encrypted", - ANY - ), - queue="bulk-email" - ) diff --git a/tests/app/clients/test_aws_ses.py b/tests/app/clients/test_aws_ses.py index adfa1ef6f..e00d65edf 100644 --- a/tests/app/clients/test_aws_ses.py +++ b/tests/app/clients/test_aws_ses.py @@ -1,5 +1,7 @@ import pytest +from unittest.mock import Mock, ANY +from app import aws_ses_client from app.clients.email.aws_ses import get_aws_responses @@ -39,3 +41,29 @@ def test_should_be_none_if_unrecognised_status_code(): with pytest.raises(KeyError) as e: get_aws_responses('99') assert '99' in str(e.value) + + +@pytest.mark.parametrize( + 'reply_to_address, expected_value', + [(None, []), ('foo@bar.com', ['foo@bar.com'])], + ids=['empty', 'single_email'] +) +def test_send_email_handles_reply_to_address(notify_api, mocker, reply_to_address, expected_value): + boto_mock = mocker.patch.object(aws_ses_client, '_client', create=True) + mocker.patch.object(aws_ses_client, 'statsd_client', create=True) + + with notify_api.app_context(): + aws_ses_client.send_email( + Mock(), + Mock(), + Mock(), + Mock(), + reply_to_address=reply_to_address + ) + + boto_mock.send_email.assert_called_once_with( + Source=ANY, + Destination=ANY, + Message=ANY, + ReplyToAddresses=expected_value + )