Merge pull request #507 from alphagov/reply-to-nondestructive-part-b

remove reply_to_addresses from task kwargs

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)

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()

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
-    )

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"
-    )

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
+    )