dont pass reply-to-addresses about

dont send reply_to_addresses around from process_job and send_email -
take it from the service in send_email_to_provider. also clean up
the kwarg in aws_ses.send_email to more accurately reflect what we
might pass in

app/celery/provider_tasks.py

@@ -148,7 +148,7 @@ def send_email_to_provider(self, service_id, notification_id, reply_to_addresses
                     template.replaced_subject,
                     body=template.replaced_govuk_escaped,
                     html_body=template.as_HTML_email,
-                    reply_to_addresses=reply_to_addresses,
+                    reply_to_address=service.reply_to_email_address,
                 )
 
                 update_provider_stats(

app/celery/tasks.py

@@ -106,7 +106,6 @@ def process_job(job_id):
                 create_uuid(),
                 encrypted,
                 datetime.utcnow().strftime(DATETIME_FORMAT)),
-                {'reply_to_addresses': service.reply_to_email_address},
                 queue='bulk-email')
 
     finished = datetime.utcnow()
@@ -182,8 +181,7 @@ def send_email(self, service_id,
     try:
         _save_notification(created_at, notification, notification_id, service_id, EMAIL_TYPE, api_key_id, key_type)
 
-        send_email_to_provider.apply_async((service_id, notification_id, reply_to_addresses),
+        send_email_to_provider.apply_async((service_id, notification_id), queue='email')
-                                           queue='email')
 
         current_app.logger.info("Email {} created at {}".format(notification_id, created_at))
         statsd_client.incr("notifications.tasks.send-email")

app/clients/email/aws_ses.py

@@ -60,14 +60,12 @@ class AwsSesClient(EmailClient):
                    subject,
                    body,
                    html_body='',
-                   reply_to_addresses=None):
+                   reply_to_address=None):
         try:
             if isinstance(to_addresses, str):
                 to_addresses = [to_addresses]
-            if reply_to_addresses and isinstance(reply_to_addresses, str):
+
-                reply_to_addresses = [reply_to_addresses]
+            reply_to_addresses = [reply_to_address] if reply_to_address else []
-            elif reply_to_addresses is None:
-                reply_to_addresses = []
 
             body = {
                 'Text': {'Data': body}

tests/app/celery/test_provider_tasks.py

@@ -2,11 +2,11 @@ import uuid
 from datetime import datetime
 
 from celery.exceptions import MaxRetriesExceededError
-from mock import ANY, call
+from unittest.mock import ANY, call
 from notifications_utils.recipients import validate_phone_number, format_phone_number
 
 import app
-from app import statsd_client, mmg_client
+from app import statsd_client, mmg_client, aws_ses_client
 from app.celery import provider_tasks
 from app.celery.provider_tasks import send_sms_to_provider, send_email_to_provider
 from app.celery.research_mode_tasks import send_sms_response, send_email_response
@@ -414,7 +414,6 @@ def test_send_email_to_provider_statsd_updates(notify_db, notify_db_session, sam
     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')
     notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session,
                                        template=sample_email_template)
     send_email_to_provider(
@@ -448,3 +447,33 @@ 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

@@ -231,7 +231,6 @@ 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")
@@ -276,7 +275,6 @@ 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)
@@ -492,8 +490,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
     statsd_client.timing.assert_called_once_with("notifications.tasks.send-email.task-time", ANY)
     persisted_notification = Notification.query.filter_by(id=notification_id).one()
     provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
-        (sample_email_template_with_placeholders.service_id,
+        (sample_email_template_with_placeholders.service_id, notification_id), queue='email')
-         notification_id, None), queue='email')
 
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == 'my_email@my_email.com'
@@ -529,9 +526,8 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email
         now.strftime(DATETIME_FORMAT)
     )
 
-    provider_tasks.send_email_to_provider.apply_async.assert_called_with((sample_email_template.service_id,
+    provider_tasks.send_email_to_provider.apply_async.assert_called_once_with((sample_email_template.service_id,
-                                                                          notification_id,
+                                                                               notification_id), queue='email')
-                                                                          None), queue='email')
 
     persisted_notification = Notification.query.filter_by(id=notification_id).one()
     assert persisted_notification.id == notification_id
@@ -558,8 +554,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi
         now.strftime(DATETIME_FORMAT)
     )
     provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
-        (sample_email_template_with_placeholders.service_id,
+        (sample_email_template_with_placeholders.service_id, notification_id, ), queue='email'
-         notification_id, None), queue='email'
     )
     persisted_notification = Notification.query.filter_by(id=notification_id).one()
     assert persisted_notification.id == notification_id
@@ -585,7 +580,7 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em
         now.strftime(DATETIME_FORMAT)
     )
     provider_tasks.send_email_to_provider.apply_async.assert_called_once_with((sample_email_template.service_id,
-                                                                               notification_id, None), queue='email')
+                                                                               notification_id), queue='email')
 
     persisted_notification = Notification.query.filter_by(id=notification_id).one()
     assert persisted_notification.id == notification_id
@@ -654,7 +649,7 @@ def test_send_email_should_go_to_retry_queue_if_database_errors(sample_email_tem
     assert 'No row was found for one' in str(e.value)
 
 
-def test_process_email_job_should_use_reply_to_email_if_present(sample_email_job, mocker, mock_celery_remove_job):
+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')
@@ -671,6 +666,5 @@ def test_process_email_job_should_use_reply_to_email_if_present(sample_email_job
             "something_encrypted",
             ANY
         ),
-        {'reply_to_addresses': 'somereply@testservice.gov.uk'},
         queue="bulk-email"
     )