Merge pull request #1433 from alphagov/add-reply-to-notifications

Populate reply-to-text field in notifications

app/celery/tasks.py

@@ -215,7 +215,8 @@ def save_sms(self,
             created_at=datetime.utcnow(),
             job_id=notification.get('job', None),
             job_row_number=notification.get('row_number', None),
-            notification_id=notification_id
+            notification_id=notification_id,
+            reply_to_text=service.get_default_sms_sender()
         )
 
         provider_tasks.deliver_sms.apply_async(
@@ -262,7 +263,8 @@ def save_email(self,
             created_at=datetime.utcnow(),
             job_id=notification.get('job', None),
             job_row_number=notification.get('row_number', None),
-            notification_id=notification_id
+            notification_id=notification_id,
+            reply_to_text=service.get_default_reply_to_email_address()
         )
 
         provider_tasks.deliver_email.apply_async(
@@ -303,7 +305,8 @@ def save_letter(
             job_id=notification['job'],
             job_row_number=notification['row_number'],
             notification_id=notification_id,
-            reference=create_random_identifier()
+            reference=create_random_identifier(),
+            reply_to_text=service.get_default_letter_contact()
         )
 
         current_app.logger.info("Letter {} created at {}".format(saved_notification.id, saved_notification.created_at))

app/invite/rest.py

@@ -42,7 +42,8 @@ def create_invited_user(service_id):
         },
         notification_type=EMAIL_TYPE,
         api_key_id=None,
-        key_type=KEY_TYPE_NORMAL
+        key_type=KEY_TYPE_NORMAL,
+        reply_to_text=service.get_default_reply_to_email_address()
     )
 
     send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY)

app/notifications/process_letter_notifications.py

@@ -3,7 +3,7 @@ from app.models import LETTER_TYPE
 from app.notifications.process_notifications import persist_notification
 
 
-def create_letter_notification(letter_data, template, api_key, status):
+def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None):
     notification = persist_notification(
         template_id=template.id,
         template_version=template.version,
@@ -18,6 +18,7 @@ def create_letter_notification(letter_data, template, api_key, status):
         job_row_number=None,
         reference=create_random_identifier(),
         client_reference=letter_data.get('reference'),
-        status=status
+        status=status,
+        reply_to_text=reply_to_text
     )
     return notification

app/notifications/process_notifications.py

@@ -54,7 +54,8 @@ def persist_notification(
     notification_id=None,
     simulated=False,
     created_by_id=None,
-    status=NOTIFICATION_CREATED
+    status=NOTIFICATION_CREATED,
+    reply_to_text=None
 ):
     notification_created_at = created_at or datetime.utcnow()
     if not notification_id:
@@ -76,7 +77,8 @@ def persist_notification(
         client_reference=client_reference,
         reference=reference,
         created_by_id=created_by_id,
-        status=status
+        status=status,
+        reply_to_text=reply_to_text,
     )
 
     if notification_type == SMS_TYPE:

app/notifications/rest.py

@@ -119,7 +119,7 @@ def send_notification(notification_type):
 
     if notification_type == SMS_TYPE:
         _service_can_send_internationally(authenticated_service, notification_form['to'])
-
+    reply_to = get_reply_to_text(notification_type)
     # Do not persist or send notification to the queue if it is a simulated recipient
     simulated = simulated_recipient(notification_form['to'], notification_type)
     notification_model = persist_notification(template_id=template.id,
@@ -130,7 +130,9 @@ def send_notification(notification_type):
                                               notification_type=notification_type,
                                               api_key_id=api_user.id,
                                               key_type=api_user.key_type,
-                                              simulated=simulated)
+                                              simulated=simulated,
+                                              reply_to_text=reply_to
+                                              )
     if not simulated:
         queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
         send_notification_to_queue(notification=notification_model,
@@ -148,6 +150,15 @@ def send_notification(notification_type):
     ), 201
 
 
+def get_reply_to_text(notification_type):
+    if notification_type == EMAIL_TYPE:
+        return authenticated_service.get_default_reply_to_email_address()
+    if notification_type == SMS_TYPE:
+        return authenticated_service.get_default_sms_sender()
+    if notification_type == LETTER_TYPE:
+        return authenticated_service.get_default_letter_contact()
+
+
 def get_notification_return_data(notification_id, notification, template):
     output = {
         'body': str(template),

app/notifications/validators.py

@@ -138,11 +138,8 @@ def validate_template(template_id, personalisation, service, notification_type):
 
 def check_service_email_reply_to_id(service_id, reply_to_id, notification_type):
     if reply_to_id:
-        if notification_type != EMAIL_TYPE:
-            message = 'email_reply_to_id is not a valid option for {} notification'.format(notification_type)
-            raise BadRequestError(message=message)
         try:
-            dao_get_reply_to_by_id(service_id, reply_to_id)
+            return dao_get_reply_to_by_id(service_id, reply_to_id).email_address
         except NoResultFound:
             message = 'email_reply_to_id {} does not exist in database for service id {}'\
                 .format(reply_to_id, service_id)
@@ -151,9 +148,6 @@ def check_service_email_reply_to_id(service_id, reply_to_id, notification_type):
 
 def check_service_sms_sender_id(service_id, sms_sender_id, notification_type):
     if sms_sender_id:
-        if notification_type != SMS_TYPE:
-            message = 'sms_sender_id is not a valid option for {} notification'.format(notification_type)
-            raise BadRequestError(message=message)
         try:
             return dao_get_service_sms_senders_by_id(service_id, sms_sender_id).sms_sender
         except NoResultFound:

app/service/send_notification.py

@@ -1,8 +1,11 @@
 from app.config import QueueNames
+from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
+from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id
 from app.notifications.validators import (
     check_service_over_daily_message_limit,
     validate_and_format_recipient,
-    validate_template)
+    validate_template
+)
 from app.notifications.process_notifications import (
     persist_notification,
     send_notification_to_queue,
@@ -13,7 +16,9 @@ from app.models import (
     KEY_TYPE_NORMAL,
     PRIORITY,
     SMS_TYPE,
-    EMAIL_TYPE)
+    EMAIL_TYPE,
+    LETTER_TYPE
+)
 from app.dao.services_dao import dao_fetch_service_by_id
 from app.dao.templates_dao import dao_get_template_by_id_and_service_id
 from app.dao.users_dao import get_user_by_id
@@ -52,6 +57,8 @@ def send_one_off_notification(service_id, post_data):
 
     validate_created_by(service, post_data['created_by'])
 
+    sender_id = post_data.get('sender_id', None)
+    reply_to = get_reply_to_text(notification_type=template.template_type, sender_id=sender_id, service=service)
     notification = persist_notification(
         template_id=template.id,
         template_version=template.version,
@@ -61,9 +68,9 @@ def send_one_off_notification(service_id, post_data):
         notification_type=template.template_type,
         api_key_id=None,
         key_type=KEY_TYPE_NORMAL,
-        created_by_id=post_data['created_by']
+        created_by_id=post_data['created_by'],
+        reply_to_text=reply_to
     )
-    sender_id = post_data.get('sender_id', None)
     if sender_id:
         if template.template_type == EMAIL_TYPE:
             persist_email_reply_to_id_for_notification(notification.id, sender_id)
@@ -78,3 +85,23 @@ def send_one_off_notification(service_id, post_data):
     )
 
     return {'id': str(notification.id)}
+
+
+def get_reply_to_text(notification_type, sender_id, service):
+    reply_to = None
+    if notification_type == EMAIL_TYPE:
+        if sender_id:
+            reply_to = dao_get_reply_to_by_id(service.id, sender_id).email_address
+        else:
+            service.get_default_reply_to_email_address()
+
+    elif notification_type == SMS_TYPE:
+        if sender_id:
+            reply_to = dao_get_service_sms_senders_by_id(service.id, sender_id).sms_sender
+        else:
+            reply_to = service.get_default_sms_sender()
+
+    elif notification_type == LETTER_TYPE:
+        reply_to = service.get_default_letter_contact()
+
+    return reply_to

app/service/sender.py

@@ -23,7 +23,8 @@ def send_notification_to_service_users(service_id, template_id, personalisation=
             personalisation=personalisation,
             notification_type=template.template_type,
             api_key_id=None,
-            key_type=KEY_TYPE_NORMAL
+            key_type=KEY_TYPE_NORMAL,
+            reply_to_text=notify_service.get_default_reply_to_email_address()
         )
         send_notification_to_queue(notification, False, queue=QueueNames.NOTIFY)
 

app/user/rest.py

@@ -214,7 +214,11 @@ def create_2fa_code(template_id, user_to_send_to, secret_code, recipient, person
 
     # save the code in the VerifyCode table
     create_user_code(user_to_send_to, secret_code, template.template_type)
-
+    reply_to = None
+    if template.template_type == SMS_TYPE:
+        reply_to = template.service.get_default_sms_sender()
+    elif template.template_type == EMAIL_TYPE:
+        reply_to = template.service.get_default_reply_to_email_address()
     saved_notification = persist_notification(
         template_id=template.id,
         template_version=template.version,
@@ -223,7 +227,8 @@ def create_2fa_code(template_id, user_to_send_to, secret_code, recipient, person
         personalisation=personalisation,
         notification_type=template.template_type,
         api_key_id=None,
-        key_type=KEY_TYPE_NORMAL
+        key_type=KEY_TYPE_NORMAL,
+        reply_to_text=reply_to
     )
     # Assume that we never want to observe the Notify service's research mode
     # setting for this notification - we still need to be able to log into the
@@ -253,7 +258,8 @@ def send_user_confirm_new_email(user_id):
         },
         notification_type=template.template_type,
         api_key_id=None,
-        key_type=KEY_TYPE_NORMAL
+        key_type=KEY_TYPE_NORMAL,
+        reply_to_text=service.get_default_reply_to_email_address()
     )
 
     send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY)
@@ -279,7 +285,8 @@ def send_new_user_email_verification(user_id):
         },
         notification_type=template.template_type,
         api_key_id=None,
-        key_type=KEY_TYPE_NORMAL
+        key_type=KEY_TYPE_NORMAL,
+        reply_to_text=service.get_default_reply_to_email_address()
     )
 
     send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY)
@@ -305,7 +312,8 @@ def send_already_registered_email(user_id):
         },
         notification_type=template.template_type,
         api_key_id=None,
-        key_type=KEY_TYPE_NORMAL
+        key_type=KEY_TYPE_NORMAL,
+        reply_to_text=service.get_default_reply_to_email_address()
     )
 
     send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY)
@@ -367,7 +375,8 @@ def send_user_reset_password():
         },
         notification_type=template.template_type,
         api_key_id=None,
-        key_type=KEY_TYPE_NORMAL
+        key_type=KEY_TYPE_NORMAL,
+        reply_to_text=service.get_default_reply_to_email_address()
     )
 
     send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY)

app/v2/notifications/notification_schemas.py

@@ -127,7 +127,8 @@ post_sms_request = {
         "scheduled_for": {"type": ["string", "null"], "format": "datetime"},
         "sms_sender_id": uuid
     },
-    "required": ["phone_number", "template_id"]
+    "required": ["phone_number", "template_id"],
+    "additionalProperties": False
 }
 
 sms_content = {
@@ -172,7 +173,8 @@ post_email_request = {
         "scheduled_for": {"type": ["string", "null"], "format": "datetime"},
         "email_reply_to_id": uuid
     },
-    "required": ["email_address", "template_id"]
+    "required": ["email_address", "template_id"],
+    "additionalProperties": False
 }
 
 email_content = {
@@ -215,7 +217,8 @@ post_letter_request = {
         "template_id": uuid,
         "personalisation": letter_personalisation
     },
-    "required": ["template_id", "personalisation"]
+    "required": ["template_id", "personalisation"],
+    "additionalProperties": False
 }
 
 letter_content = {

app/v2/notifications/post_notifications.py

@@ -63,15 +63,12 @@ def post_notification(notification_type):
     check_service_has_permission(notification_type, authenticated_service.permissions)
 
     scheduled_for = form.get("scheduled_for", None)
-    service_email_reply_to_id = form.get("email_reply_to_id", None)
-    service_sms_sender_id = form.get("sms_sender_id", None)
 
     check_service_can_schedule_notification(authenticated_service.permissions, scheduled_for)
 
     check_rate_limiting(authenticated_service, api_user)
 
-    check_service_email_reply_to_id(str(authenticated_service.id), service_email_reply_to_id, notification_type)
+    reply_to = get_reply_to_text(notification_type, form)
-    sms_sender = check_service_sms_sender_id(str(authenticated_service.id), service_sms_sender_id, notification_type)
 
     template, template_with_content = validate_template(
         form['template_id'],
@@ -92,13 +89,14 @@ def post_notification(notification_type):
             notification_type=notification_type,
             api_key=api_user,
             template=template,
-            service=authenticated_service
+            service=authenticated_service,
+            reply_to_text=reply_to
         )
 
     if notification_type == SMS_TYPE:
         create_resp_partial = functools.partial(
             create_post_sms_response_from_notification,
-            from_number=sms_sender or authenticated_service.get_default_sms_sender()
+            from_number=reply_to
         )
     elif notification_type == EMAIL_TYPE:
         create_resp_partial = functools.partial(
@@ -121,7 +119,7 @@ def post_notification(notification_type):
     return jsonify(resp), 201
 
 
-def process_sms_or_email_notification(*, form, notification_type, api_key, template, service):
+def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None):
     form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number']
 
     send_to = validate_and_format_recipient(send_to=form_send_to,
@@ -142,7 +140,8 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
         api_key_id=api_key.id,
         key_type=api_key.key_type,
         client_reference=form.get('reference', None),
-        simulated=simulated
+        simulated=simulated,
+        reply_to_text=reply_to_text
     )
 
     persist_sender_to_notification_mapping(form, notification)
@@ -184,8 +183,12 @@ def process_letter_notification(*, letter_data, api_key, template):
 
     # if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up
     status = NOTIFICATION_CREATED if should_send else NOTIFICATION_SENDING
-
+    letter_contact_block = api_key.service.get_default_letter_contact()
-    notification = create_letter_notification(letter_data, template, api_key, status=status)
+    notification = create_letter_notification(letter_data=letter_data,
+                                              template=template,
+                                              api_key=api_key,
+                                              status=status,
+                                              reply_to_text=letter_contact_block)
 
     if not should_send:
         update_letter_notifications_to_sent_to_dvla.apply_async(
@@ -194,3 +197,23 @@ def process_letter_notification(*, letter_data, api_key, template):
         )
 
     return notification
+
+
+def get_reply_to_text(notification_type, form):
+    reply_to = None
+    if notification_type == EMAIL_TYPE:
+        service_email_reply_to_id = form.get("email_reply_to_id", None)
+        reply_to = check_service_email_reply_to_id(
+            str(authenticated_service.id), service_email_reply_to_id, notification_type
+        ) or authenticated_service.get_default_reply_to_email_address()
+
+    elif notification_type == SMS_TYPE:
+        service_sms_sender_id = form.get("sms_sender_id", None)
+        reply_to = check_service_sms_sender_id(
+            str(authenticated_service.id), service_sms_sender_id, notification_type
+        ) or authenticated_service.get_default_sms_sender()
+
+    elif notification_type == LETTER_TYPE:
+        reply_to = authenticated_service.get_default_letter_contact()
+
+    return reply_to

environment_test.sh

@@ -12,3 +12,6 @@ export FIRETEXT_API_KEY="Firetext"
 export STATSD_PREFIX="stats-prefix"
 export NOTIFICATION_QUEUE_PREFIX='testing'
 export REDIS_URL="redis://localhost:6379/0"
+export FLASK_APP=application.py
+export FLASK_DEBUG=1
+export WERKZEUG_DEBUG_PIN=off

tests/app/celery/test_tasks.py

@@ -52,7 +52,8 @@ from tests.app.conftest import (
     sample_template as create_sample_template,
     sample_job as create_sample_job,
     sample_email_template as create_sample_email_template,
-    sample_notification as create_sample_notification
+    sample_notification as create_sample_notification,
+    sample_letter_job
 )
 from tests.app.db import (
     create_inbound_sms,
@@ -62,7 +63,9 @@ from tests.app.db import (
     create_service_inbound_api,
     create_service,
     create_template,
-    create_user
+    create_user,
+    create_reply_to_email,
+    create_service_with_defined_sms_sender
 )
 
 
@@ -542,6 +545,45 @@ def test_should_save_email_if_restricted_service_and_non_team_email_address_with
     )
 
 
+def test_save_email_should_save_default_email_reply_to_text_on_notification(notify_db_session, mocker):
+    service = create_service()
+    create_reply_to_email(service=service, email_address='reply_to@digital.gov.uk', is_default=True)
+    template = create_template(service=service, template_type='email', subject='Hello')
+
+    notification = _notification_json(template, to="test@example.com")
+    mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+
+    notification_id = uuid.uuid4()
+    save_email(
+        service.id,
+        notification_id,
+        encryption.encrypt(notification),
+        key_type=KEY_TYPE_TEST
+    )
+
+    persisted_notification = Notification.query.one()
+    assert persisted_notification.reply_to_text == 'reply_to@digital.gov.uk'
+
+
+def test_save_sms_should_save_default_smm_sender_notification_reply_to_text_on(notify_db_session, mocker):
+    service = create_service_with_defined_sms_sender(sms_sender_value='12345')
+    template = create_template(service=service)
+
+    notification = _notification_json(template, to="07700 900205")
+    mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+
+    notification_id = uuid.uuid4()
+    save_sms(
+        service.id,
+        notification_id,
+        encryption.encrypt(notification),
+        key_type=KEY_TYPE_TEST
+    )
+
+    persisted_notification = Notification.query.one()
+    assert persisted_notification.reply_to_text == '12345'
+
+
 def test_should_not_save_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker):
     user = create_user(mobile_number="07700 900205")
     service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True)
@@ -948,7 +990,11 @@ def test_save_sms_does_not_send_duplicate_and_does_not_put_in_retry_queue(sample
     assert not retry.called
 
 
-def test_save_letter_saves_letter_to_database(sample_letter_job, mocker):
+def test_save_letter_saves_letter_to_database(mocker, notify_db_session):
+    service = create_service()
+    create_letter_contact(service=service, contact_block="Address contact", is_default=True)
+    template = create_template(service=service, template_type=LETTER_TYPE)
+    job = create_job(template=template)
 
     mocker.patch('app.celery.tasks.create_random_identifier', return_value="this-is-random-in-real-life")
 
@@ -962,17 +1008,17 @@ def test_save_letter_saves_letter_to_database(sample_letter_job, mocker):
         'postcode': 'Flob',
     }
     notification_json = _notification_json(
-        template=sample_letter_job.template,
+        template=job.template,
         to='Foo',
         personalisation=personalisation,
-        job_id=sample_letter_job.id,
+        job_id=job.id,
         row_number=1
     )
     notification_id = uuid.uuid4()
     created_at = datetime.utcnow()
 
     save_letter(
-        sample_letter_job.service_id,
+        job.service_id,
         notification_id,
         encryption.encrypt(notification_json),
     )
@@ -980,9 +1026,9 @@ def test_save_letter_saves_letter_to_database(sample_letter_job, mocker):
     notification_db = Notification.query.one()
     assert notification_db.id == notification_id
     assert notification_db.to == 'Foo'
-    assert notification_db.job_id == sample_letter_job.id
+    assert notification_db.job_id == job.id
-    assert notification_db.template_id == sample_letter_job.template.id
+    assert notification_db.template_id == job.template.id
-    assert notification_db.template_version == sample_letter_job.template.version
+    assert notification_db.template_version == job.template.version
     assert notification_db.status == 'created'
     assert notification_db.created_at >= created_at
     assert notification_db.notification_type == 'letter'
@@ -990,6 +1036,7 @@ def test_save_letter_saves_letter_to_database(sample_letter_job, mocker):
     assert notification_db.sent_by is None
     assert notification_db.personalisation == personalisation
     assert notification_db.reference == "this-is-random-in-real-life"
+    assert notification_db.reply_to_text == "Address contact"
 
 
 def test_should_cancel_job_if_service_is_inactive(sample_service,

tests/app/conftest.py

@@ -27,9 +27,18 @@ from app.models import (
     ProviderRates,
     ScheduledNotification,
     ServiceWhitelist,
-    KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM,
+    KEY_TYPE_NORMAL,
-    MOBILE_TYPE, EMAIL_TYPE, INBOUND_SMS_TYPE, SMS_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED,
+    KEY_TYPE_TEST,
-    SERVICE_PERMISSION_TYPES)
+    KEY_TYPE_TEAM,
+    MOBILE_TYPE,
+    EMAIL_TYPE,
+    INBOUND_SMS_TYPE,
+    SMS_TYPE,
+    LETTER_TYPE,
+    NOTIFICATION_STATUS_TYPES_COMPLETED,
+    SERVICE_PERMISSION_TYPES,
+    ServiceEmailReplyTo
+)
 from app.dao.users_dao import (create_user_code, create_secret_code)
 from app.dao.services_dao import (dao_create_service, dao_add_user_to_service)
 from app.dao.templates_dao import dao_create_template
@@ -985,17 +994,26 @@ def notify_service(notify_db, notify_db_session):
     user = create_user()
     service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID'])
     if not service:
+        service = Service(
+            name='Notify Service',
+            message_limit=1000,
+            restricted=False,
+            email_from='notify.service',
+            created_by=user,
+            prefix_sms=False,
+        )
+        dao_create_service(service=service, service_id=current_app.config['NOTIFY_SERVICE_ID'], user=user)
+
         data = {
-            'id': current_app.config['NOTIFY_SERVICE_ID'],
+            'service': service,
-            'name': 'Notify Service',
+            'email_address': "notify@gov.uk",
-            'message_limit': 1000,
+            'is_default': True,
-            'active': True,
-            'restricted': False,
-            'email_from': 'notify.service',
-            'created_by': user,
         }
-        service = Service(**data)
+        reply_to = ServiceEmailReplyTo(**data)
-        db.session.add(service)
+
+        db.session.add(reply_to)
+        db.session.commit()
+
     return service, user
 
 

tests/app/db.py

@@ -69,10 +69,11 @@ def create_service(
     active=True,
     email_from=None,
     prefix_sms=True,
+    message_limit=1000
 ):
     service = Service(
         name=service_name,
-        message_limit=1000,
+        message_limit=message_limit,
         restricted=restricted,
         email_from=email_from if email_from else service_name.lower().replace(' ', '.'),
         created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())),

tests/app/invite/test_invite_rest.py

@@ -33,6 +33,7 @@ def test_create_invited_user(admin_request, sample_service, mocker, invitation_e
     assert json_resp['data']['id']
 
     notification = Notification.query.first()
+    assert notification.reply_to_text == "notify@gov.uk"
     mocked.assert_called_once_with([(str(notification.id))], queue="notify-internal-tasks")
 
 

tests/app/notifications/rest/test_send_notification.py

@@ -31,6 +31,7 @@ from tests.app.conftest import (
 
 from app.models import Template
 from app.errors import InvalidRequest
+from tests.app.db import create_service, create_reply_to_email
 
 
 @pytest.mark.parametrize('template_type',
@@ -1219,3 +1220,34 @@ def test_should_throw_exception_if_notification_type_is_invalid(client, sample_s
         headers=[('Content-Type', 'application/json'), auth_header])
     assert response.status_code == 400
     assert json.loads(response.get_data(as_text=True))["message"] == err_msg
+
+
+@pytest.mark.parametrize("notification_type, recipient",
+                         [("sms", '07700 900 855'),
+                          ("email", "test@gov.uk")
+                          ]
+                         )
+def test_post_notification_should_set_reply_to_text(client, notify_db, notify_db_session, mocker, notification_type,
+                                                    recipient):
+    mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type))
+    service = create_service()
+    template = create_sample_template(notify_db=notify_db, notify_db_session=notify_db_session,
+                                      service=service, template_type=notification_type)
+    expected_reply_to = current_app.config['FROM_NUMBER']
+    if notification_type == EMAIL_TYPE:
+        expected_reply_to = 'reply_to@gov.uk'
+        create_reply_to_email(service=service, email_address=expected_reply_to, is_default=True)
+
+    data = {
+        'to': recipient,
+        'template': str(template.id)
+    }
+    response = client.post("/notifications/{}".format(notification_type),
+                           data=json.dumps(data),
+                           headers=[('Content-Type', 'application/json'),
+                                    create_authorization_header(service_id=service.id)]
+                           )
+    assert response.status_code == 201
+    notifications = Notification.query.all()
+    assert len(notifications) == 1
+    assert notifications[0].reply_to_text == expected_reply_to

tests/app/notifications/test_process_notification.py

@@ -73,7 +73,8 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api
         key_type=sample_api_key.key_type,
         job_id=sample_job.id,
         job_row_number=100,
-        reference="ref")
+        reference="ref",
+        reply_to_text=sample_template.service.get_default_sms_sender())
 
     assert Notification.query.get(notification.id) is not None
     assert NotificationHistory.query.get(notification.id) is not None
@@ -97,6 +98,7 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api
     assert notification_from_db.reference == notification_history_from_db.reference
     assert notification_from_db.client_reference == notification_history_from_db.client_reference
     assert notification_from_db.created_by_id == notification_history_from_db.created_by_id
+    assert notification_from_db.reply_to_text == sample_template.service.get_default_sms_sender()
 
     mocked_redis.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count")
 
@@ -205,6 +207,7 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker)
     assert persisted_notification.phone_prefix == '44'
     assert persisted_notification.rate_multiplier == 1
     assert persisted_notification.created_by_id == sample_job.created_by_id
+    assert not persisted_notification.reply_to_text
 
 
 @freeze_time("2016-01-01 11:09:00.061258")

tests/app/notifications/test_validators.py

@@ -338,7 +338,7 @@ def test_check_service_email_reply_to_id_where_reply_to_id_is_none(notification_
 
 def test_check_service_email_reply_to_where_email_reply_to_is_found(sample_service):
     reply_to_address = create_reply_to_email(sample_service, "test@test.com")
-    assert check_service_email_reply_to_id(sample_service.id, reply_to_address.id, EMAIL_TYPE) is None
+    assert check_service_email_reply_to_id(sample_service.id, reply_to_address.id, EMAIL_TYPE) == "test@test.com"
 
 
 def test_check_service_email_reply_to_id_where_service_id_is_not_found(sample_service, fake_uuid):
@@ -358,15 +358,6 @@ def test_check_service_email_reply_to_id_where_reply_to_id_is_not_found(sample_s
         .format(fake_uuid, sample_service.id)
 
 
-@pytest.mark.parametrize('notification_type', ['sms', 'letter'])
-def test_check_service_email_reply_to_id_when_channel_type_is_wrong(sample_service, notification_type):
-    reply_to_address = create_reply_to_email(sample_service, "test@test.com")
-    with pytest.raises(BadRequestError) as e:
-        check_service_email_reply_to_id(sample_service.id, reply_to_address.id, notification_type)
-    assert e.value.status_code == 400
-    assert e.value.message == 'email_reply_to_id is not a valid option for {} notification'.format(notification_type)
-
-
 @pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter'])
 def test_check_service_sms_sender_id_where_sms_sender_id_is_none(notification_type):
     assert check_service_sms_sender_id(None, None, notification_type) is None
@@ -392,12 +383,3 @@ def test_check_service_sms_sender_id_where_sms_sender_is_not_found(sample_servic
     assert e.value.status_code == 400
     assert e.value.message == 'sms_sender_id {} does not exist in database for service id {}' \
         .format(fake_uuid, sample_service.id)
-
-
-@pytest.mark.parametrize('notification_type', ['email', 'letter'])
-def test_check_service_sms_sender_id_when_channel_type_is_wrong(sample_service, notification_type):
-    sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456')
-    with pytest.raises(BadRequestError) as e:
-        check_service_sms_sender_id(sample_service.id, sms_sender.id, notification_type)
-    assert e.value.status_code == 400
-    assert e.value.message == 'sms_sender_id is not a valid option for {} notification'.format(notification_type)

tests/app/service/test_rest.py

@@ -2303,16 +2303,18 @@ def test_fetch_service_inbound_api(client, sample_service):
     assert json.loads(response.get_data(as_text=True))["data"] == service_inbound_api.serialize()
 
 
-def test_send_one_off_notification(admin_request, sample_template, mocker):
+def test_send_one_off_notification(admin_request, mocker):
+    service = create_service()
+    template = create_template(service=service)
     mocker.patch('app.service.send_notification.send_notification_to_queue')
 
     response = admin_request.post(
         'service.create_one_off_notification',
-        service_id=sample_template.service_id,
+        service_id=service.id,
         _data={
-            'template_id': str(sample_template.id),
+            'template_id': str(template.id),
             'to': '07700900001',
-            'created_by': str(sample_template.service.created_by_id)
+            'created_by': str(service.created_by_id)
         },
         _expected_status=201
     )

tests/app/service/test_send_one_off_notification.py

@@ -17,7 +17,13 @@ from app.models import (
     NotificationSmsSender
 )
 
-from tests.app.db import create_user, create_reply_to_email, create_service_sms_sender
+from tests.app.db import (
+    create_user,
+    create_reply_to_email,
+    create_service_sms_sender,
+    create_service,
+    create_template
+)
 
 
 @pytest.fixture
@@ -31,11 +37,14 @@ def celery_mock(mocker):
     return mocker.patch('app.service.send_notification.send_notification_to_queue')
 
 
-def test_send_one_off_notification_calls_celery_correctly(persist_mock, celery_mock, sample_template):
+def test_send_one_off_notification_calls_celery_correctly(persist_mock, celery_mock, notify_db_session):
-    service = sample_template.service
+    service = create_service()
+    template = create_template(service=service)
+
+    service = template.service
 
     post_data = {
-        'template_id': str(sample_template.id),
+        'template_id': str(template.id),
         'to': '07700 900 001',
         'created_by': str(service.created_by_id)
     }
@@ -56,10 +65,10 @@ def test_send_one_off_notification_calls_celery_correctly(persist_mock, celery_m
 def test_send_one_off_notification_calls_persist_correctly(
     persist_mock,
     celery_mock,
-    sample_template_with_placeholders
+    notify_db_session
 ):
-    template = sample_template_with_placeholders
+    service = create_service()
-    service = template.service
+    template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon")
 
     post_data = {
         'template_id': str(template.id),
@@ -79,16 +88,17 @@ def test_send_one_off_notification_calls_persist_correctly(
         notification_type=SMS_TYPE,
         api_key_id=None,
         key_type=KEY_TYPE_NORMAL,
-        created_by_id=str(service.created_by_id)
+        created_by_id=str(service.created_by_id),
+        reply_to_text='testing'
     )
 
 
-def test_send_one_off_notification_honors_research_mode(persist_mock, celery_mock, sample_template):
+def test_send_one_off_notification_honors_research_mode(notify_db_session, persist_mock, celery_mock):
-    service = sample_template.service
+    service = create_service(research_mode=True)
-    service.research_mode = True
+    template = create_template(service=service)
 
     post_data = {
-        'template_id': str(sample_template.id),
+        'template_id': str(template.id),
         'to': '07700 900 001',
         'created_by': str(service.created_by_id)
     }
@@ -98,12 +108,13 @@ def test_send_one_off_notification_honors_research_mode(persist_mock, celery_moc
     assert celery_mock.call_args[1]['research_mode'] is True
 
 
-def test_send_one_off_notification_honors_priority(persist_mock, celery_mock, sample_template):
+def test_send_one_off_notification_honors_priority(notify_db_session, persist_mock, celery_mock):
-    service = sample_template.service
+    service = create_service()
-    sample_template.process_type = PRIORITY
+    template = create_template(service=service)
+    template.process_type = PRIORITY
 
     post_data = {
-        'template_id': str(sample_template.id),
+        'template_id': str(template.id),
         'to': '07700 900 001',
         'created_by': str(service.created_by_id)
     }
@@ -113,11 +124,12 @@ def test_send_one_off_notification_honors_priority(persist_mock, celery_mock, sa
     assert celery_mock.call_args[1]['queue'] == QueueNames.PRIORITY
 
 
-def test_send_one_off_notification_raises_if_invalid_recipient(sample_template):
+def test_send_one_off_notification_raises_if_invalid_recipient(notify_db_session):
-    service = sample_template.service
+    service = create_service()
+    template = create_template(service=service)
 
     post_data = {
-        'template_id': str(sample_template.id),
+        'template_id': str(template.id),
         'to': 'not a phone number',
         'created_by': str(service.created_by_id)
     }
@@ -126,12 +138,12 @@ def test_send_one_off_notification_raises_if_invalid_recipient(sample_template):
         send_one_off_notification(service.id, post_data)
 
 
-def test_send_one_off_notification_raises_if_cant_send_to_recipient(sample_template):
+def test_send_one_off_notification_raises_if_cant_send_to_recipient(notify_db_session):
-    service = sample_template.service
+    service = create_service(restricted=True)
-    service.restricted = True
+    template = create_template(service=service)
 
     post_data = {
-        'template_id': str(sample_template.id),
+        'template_id': str(template.id),
         'to': '07700 900 001',
         'created_by': str(service.created_by_id)
     }
@@ -142,12 +154,12 @@ def test_send_one_off_notification_raises_if_cant_send_to_recipient(sample_templ
     assert 'service is in trial mode' in e.value.message
 
 
-def test_send_one_off_notification_raises_if_over_limit(sample_template):
+def test_send_one_off_notification_raises_if_over_limit(notify_db_session):
-    service = sample_template.service
+    service = create_service(message_limit=0)
-    service.message_limit = 0
+    template = create_template(service=service)
 
     post_data = {
-        'template_id': str(sample_template.id),
+        'template_id': str(template.id),
         'to': '07700 900 001',
         'created_by': str(service.created_by_id)
     }
@@ -156,9 +168,9 @@ def test_send_one_off_notification_raises_if_over_limit(sample_template):
         send_one_off_notification(service.id, post_data)
 
 
-def test_send_one_off_notification_raises_if_message_too_long(persist_mock, sample_template_with_placeholders):
+def test_send_one_off_notification_raises_if_message_too_long(persist_mock, notify_db_session):
-    template = sample_template_with_placeholders
+    service = create_service()
-    service = template.service
+    template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon")
 
     post_data = {
         'template_id': str(template.id),

tests/app/service/test_sender.py

@@ -26,7 +26,7 @@ def test_send_notification_to_service_users_persists_notifications_correctly(
 ):
     mocker.patch('app.service.sender.send_notification_to_queue')
 
-    create_notify_service(notify_db, notify_db_session)
+    notify_service, user = create_notify_service(notify_db, notify_db_session)
     service = create_sample_service(notify_db, notify_db_session, user=sample_user)
     template = create_template(service, template_type=notification_type)
     send_notification_to_service_users(service_id=service.id, template_id=template.id)
@@ -40,6 +40,7 @@ def test_send_notification_to_service_users_persists_notifications_correctly(
     assert notification.template.id == template.id
     assert notification.template.template_type == notification_type
     assert notification.notification_type == notification_type
+    assert notification.reply_to_text == notify_service.get_default_reply_to_email_address()
 
 
 def test_send_notification_to_service_users_sends_to_queue(

tests/app/user/test_rest.py

@@ -347,6 +347,7 @@ def test_send_user_reset_password_should_send_reset_password_link(client,
     mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
     data = json.dumps({'email': sample_user.email_address})
     auth_header = create_authorization_header()
+    notify_service = password_reset_email_template.service
     resp = client.post(
         url_for('user.send_user_reset_password'),
         data=data,
@@ -355,6 +356,7 @@ def test_send_user_reset_password_should_send_reset_password_link(client,
     assert resp.status_code == 204
     notification = Notification.query.first()
     mocked.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks")
+    assert notification.reply_to_text == notify_service.get_default_reply_to_email_address()
 
 
 def test_send_user_reset_password_should_return_400_when_email_is_missing(client, mocker):
@@ -408,6 +410,7 @@ def test_send_already_registered_email(client, sample_user, already_registered_t
     data = json.dumps({'email': sample_user.email_address})
     auth_header = create_authorization_header()
     mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+    notify_service = already_registered_template.service
 
     resp = client.post(
         url_for('user.send_already_registered_email', user_id=str(sample_user.id)),
@@ -417,6 +420,7 @@ def test_send_already_registered_email(client, sample_user, already_registered_t
 
     notification = Notification.query.first()
     mocked.assert_called_once_with(([str(notification.id)]), queue="notify-internal-tasks")
+    assert notification.reply_to_text == notify_service.get_default_reply_to_email_address()
 
 
 def test_send_already_registered_email_returns_400_when_data_is_missing(client, sample_user):
@@ -436,6 +440,7 @@ def test_send_user_confirm_new_email_returns_204(client, sample_user, change_ema
     new_email = 'new_address@dig.gov.uk'
     data = json.dumps({'email': new_email})
     auth_header = create_authorization_header()
+    notify_service = change_email_confirmation_template.service
 
     resp = client.post(url_for('user.send_user_confirm_new_email', user_id=str(sample_user.id)),
                        data=data,
@@ -445,6 +450,7 @@ def test_send_user_confirm_new_email_returns_204(client, sample_user, change_ema
     mocked.assert_called_once_with(
         ([str(notification.id)]),
         queue="notify-internal-tasks")
+    assert notification.reply_to_text == notify_service.get_default_reply_to_email_address()
 
 
 def test_send_user_confirm_new_email_returns_400_when_email_missing(client, sample_user, mocker):

tests/app/user/test_rest_verify.py

@@ -175,8 +175,8 @@ def test_send_user_sms_code(client,
     """
     Tests POST endpoint /user/<user_id>/sms-code
     """
+    notify_service = dao_fetch_service_by_id(current_app.config['NOTIFY_SERVICE_ID'])
     if research_mode:
-        notify_service = dao_fetch_service_by_id(current_app.config['NOTIFY_SERVICE_ID'])
         notify_service.research_mode = True
         dao_update_service(notify_service)
 
@@ -197,6 +197,7 @@ def test_send_user_sms_code(client,
     assert notification.personalisation == {'verify_code': '11111'}
     assert notification.to == sample_user.mobile_number
     assert str(notification.service_id) == current_app.config['NOTIFY_SERVICE_ID']
+    assert notification.reply_to_text == notify_service.get_default_sms_sender()
 
     app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
         ([str(notification.id)]),
@@ -274,10 +275,12 @@ def test_send_new_user_email_verification(client,
         url_for('user.send_new_user_email_verification', user_id=str(sample_user.id)),
         data=json.dumps({}),
         headers=[('Content-Type', 'application/json'), auth_header])
+    notify_service = email_verification_template.service
     assert resp.status_code == 204
     notification = Notification.query.first()
     assert VerifyCode.query.count() == 0
     mocked.assert_called_once_with(([str(notification.id)]), queue="notify-internal-tasks")
+    assert notification.reply_to_text == notify_service.get_default_reply_to_email_address()
 
 
 def test_send_email_verification_returns_404_for_bad_input_data(client, notify_db_session, mocker):
@@ -354,6 +357,7 @@ def test_send_user_email_code(admin_request, mocker, sample_user, email_2fa_code
         _expected_status=204
     )
     noti = Notification.query.one()
+    assert noti.reply_to_text == email_2fa_code_template.service.get_default_reply_to_email_address()
     assert noti.to == sample_user.email_address
     assert str(noti.template_id) == current_app.config['EMAIL_2FA_TEMPLATE_ID']
     assert noti.personalisation['name'] == 'Test User'

tests/app/v2/notifications/test_notification_schemas.py

@@ -237,7 +237,6 @@ def valid_email_response():
 @freeze_time("2017-05-12 13:00:00")
 def test_post_schema_valid_scheduled_for(schema):
     j = {"template_id": str(uuid.uuid4()),
-         "email_address": "joe@gmail.com",
          "scheduled_for": "2017-05-12 13:15"}
     if schema == post_email_request_schema:
         j.update({"email_address": "joe@gmail.com"})

tests/app/v2/notifications/test_post_letter_notifications.py

@@ -18,8 +18,7 @@ from app.v2.errors import RateLimitError
 from app.v2.notifications.notification_schemas import post_letter_response
 
 from tests import create_authorization_header
-from tests.app.db import create_service, create_template
+from tests.app.db import create_service, create_template, create_letter_contact
-
 
 test_address = {
     'address_line_1': 'test 1',
@@ -78,6 +77,7 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo
         ) in resp_json['template']['uri']
     )
     assert not resp_json['scheduled_for']
+    assert not notification.reply_to_text
 
 
 def test_post_letter_notification_returns_400_and_missing_template(
@@ -298,3 +298,21 @@ def test_post_letter_notification_fakes_dvla_when_service_is_in_trial_mode_but_u
         kwargs={'notification_references': [notification.reference]},
         queue='research-mode-tasks'
     )
+
+
+def test_post_letter_notification_persists_notification_reply_to_text(
+    client, notify_db_session
+):
+    service = create_service(service_permissions=[LETTER_TYPE])
+    service_address = "12 Main Street, London"
+    create_letter_contact(service=service, contact_block=service_address, is_default=True)
+    template = create_template(service=service, template_type='letter')
+    data = {
+        "template_id": template.id,
+        "personalisation": {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'}
+    }
+    letter_request(client, data=data, service_id=service.id, key_type=KEY_TYPE_NORMAL)
+
+    notifications = Notification.query.all()
+    assert len(notifications) == 1
+    assert notifications[0].reply_to_text == service_address

tests/app/v2/notifications/test_post_notifications.py

@@ -3,14 +3,14 @@ import uuid
 import pytest
 from freezegun import freeze_time
 
-from app.dao.service_sms_sender_dao import update_existing_sms_sender_with_inbound_number
+from app.dao.service_sms_sender_dao import dao_update_service_sms_sender
 from app.models import (
     NotificationEmailReplyTo,
     ScheduledNotification,
     SCHEDULE_NOTIFICATIONS,
     EMAIL_TYPE,
     SMS_TYPE,
-    NotificationSmsSender, ServiceSmsSender
+    NotificationSmsSender
 )
 from flask import json, current_app
 
@@ -31,7 +31,7 @@ from tests.app.db import (
     create_service,
     create_template,
     create_reply_to_email,
-    create_service_sms_sender, create_notification, create_inbound_number,
+    create_service_sms_sender, create_notification,
     create_service_with_inbound_number
 )
 
@@ -96,13 +96,13 @@ def test_post_sms_notification_uses_inbound_number_as_sender(client, notify_db_s
     notification_id = notifications[0].id
     assert resp_json['id'] == str(notification_id)
     assert resp_json['content']['from_number'] == '1'
+    assert notifications[0].reply_to_text == '1'
     mocked.assert_called_once_with([str(notification_id)], queue='send-sms-tasks')
 
 
 def test_post_sms_notification_returns_201_with_sms_sender_id(
         client, sample_template_with_placeholders, mocker
 ):
-
     sms_sender = create_service_sms_sender(service=sample_template_with_placeholders.service, sms_sender='123456')
     mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
     data = {
@@ -125,9 +125,40 @@ def test_post_sms_notification_returns_201_with_sms_sender_id(
     assert str(notification_to_sms_sender[0].notification_id) == resp_json['id']
     assert resp_json['content']['from_number'] == sms_sender.sms_sender
     assert notification_to_sms_sender[0].service_sms_sender_id == sms_sender.id
+    notifications = Notification.query.all()
+    assert len(notifications) == 1
+    assert notifications[0].reply_to_text == sms_sender.sms_sender
     mocked.assert_called_once_with([resp_json['id']], queue='send-sms-tasks')
 
 
+def test_notification_reply_to_text_is_original_value_if_sender_is_changed_after_post_notification(
+        client, sample_template, mocker
+):
+    sms_sender = create_service_sms_sender(service=sample_template.service, sms_sender='123456', is_default=False)
+    mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+    data = {
+        'phone_number': '+447700900855',
+        'template_id': str(sample_template.id),
+        'sms_sender_id': str(sms_sender.id)
+    }
+    auth_header = create_authorization_header(service_id=sample_template.service_id)
+
+    response = client.post(
+        path='/v2/notifications/sms',
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header])
+
+    dao_update_service_sms_sender(service_id=sample_template.service_id,
+                                  service_sms_sender_id=sms_sender.id,
+                                  is_default=sms_sender.is_default,
+                                  sms_sender='updated')
+
+    assert response.status_code == 201
+    notifications = Notification.query.all()
+    assert len(notifications) == 1
+    assert notifications[0].reply_to_text == '123456'
+
+
 @pytest.mark.parametrize("notification_type, key_send_to, send_to",
                          [("sms", "phone_number", "+447700900855"),
                           ("email", "email_address", "sample@email.com")])
@@ -198,9 +229,13 @@ def test_notification_returns_400_and_for_schema_problems(client, sample_templat
     assert response.headers['Content-type'] == 'application/json'
     error_resp = json.loads(response.get_data(as_text=True))
     assert error_resp['status_code'] == 400
-    assert error_resp['errors'] == [{'error': 'ValidationError',
+    assert {'error': 'ValidationError',
-                                     'message': "template_id is a required property"
+            'message': "template_id is a required property"
-                                     }]
+            } in error_resp['errors']
+    assert {'error': 'ValidationError',
+            'message':
+            'Additional properties are not allowed (template was unexpected)'
+            } in error_resp['errors']
 
 
 @pytest.mark.parametrize("reference", [None, "reference_from_client"])
@@ -225,6 +260,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_
     assert resp_json['id'] == str(notification.id)
     assert resp_json['reference'] == reference
     assert notification.reference is None
+    assert notification.reply_to_text is None
     assert resp_json['content']['body'] == sample_email_template_with_placeholders.content \
         .replace('((name))', 'Bob').replace('GOV.UK', u'GOV.\u200bUK')
     assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject \
@@ -291,7 +327,6 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
                                                                                    notification_type,
                                                                                    key_send_to,
                                                                                    send_to):
-
     mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type))
 
     sample = create_sample_template(
@@ -325,13 +360,13 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
     [("sms", "phone_number", "07700 900 855"), ("email", "email_address", "sample@email.com")]
 )
 def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
-    client,
+        client,
-    notify_db,
+        notify_db,
-    notify_db_session,
+        notify_db_session,
-    mocker,
+        mocker,
-    notification_type,
+        notification_type,
-    key_send_to,
+        key_send_to,
-    send_to
+        send_to
 ):
     sample = create_sample_template(
         notify_db,
@@ -369,11 +404,10 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
 
 
 def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(
-    client,
+        client,
-    notify_db,
+        notify_db,
-    notify_db_session,
+        notify_db_session,
 ):
-
     service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE])
     template = create_sample_template(notify_db, notify_db_session, service=service)
 
@@ -399,15 +433,14 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(
     ]
 
 
-@pytest.mark.parametrize('template_factory,expected_error', [
+@pytest.mark.parametrize('recipient,label,template_factory,expected_error', [
-    (sample_template_without_sms_permission, 'Cannot send text messages'),
+    ('07700 900000', 'phone_number', sample_template_without_sms_permission, 'Cannot send text messages'),
-    (sample_template_without_email_permission, 'Cannot send emails')])
+    ('someone@test.com', 'email_address', sample_template_without_email_permission, 'Cannot send emails')])
 def test_post_sms_notification_returns_400_if_not_allowed_to_send_notification(
-        client, template_factory, expected_error, notify_db, notify_db_session):
+        client, template_factory, recipient, label, expected_error, notify_db, notify_db_session):
     sample_template_without_permission = template_factory(notify_db, notify_db_session)
     data = {
-        'phone_number': '07700 900000',
+        label: recipient,
-        'email_address': 'someone@test.com',
         'template_id': sample_template_without_permission.id
     }
     auth_header = create_authorization_header(service_id=sample_template_without_permission.service.id)
@@ -428,12 +461,11 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_notification(
 
 
 def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(
-    sample_service,
+        sample_service,
-    sample_template,
+        sample_template,
-    client,
+        client,
-    mocker,
+        mocker,
 ):
-
     mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
 
     data = {
@@ -568,9 +600,9 @@ def test_post_notification_with_wrong_type_of_sender(
         headers=[('Content-Type', 'application/json'), auth_header])
     assert response.status_code == 400
     resp_json = json.loads(response.get_data(as_text=True))
-    assert '{} is not a valid option for {} notification'.\
+    assert 'Additional properties are not allowed ({} was unexpected)'.format(form_label) \
-        format(form_label, notification_type) in resp_json['errors'][0]['message']
+           in resp_json['errors'][0]['message']
-    assert 'BadRequestError' in resp_json['errors'][0]['error']
+    assert 'ValidationError' in resp_json['errors'][0]['error']
 
 
 def test_post_email_notification_with_valid_reply_to_id_returns_201(client, sample_email_template, mocker):
@@ -590,6 +622,7 @@ def test_post_email_notification_with_valid_reply_to_id_returns_201(client, samp
     resp_json = json.loads(response.get_data(as_text=True))
     assert validate(resp_json, post_email_response) == resp_json
     notification = Notification.query.first()
+    assert notification.reply_to_text == 'test@test.com'
     assert resp_json['id'] == str(notification.id)
     assert mocked.called
 
@@ -608,7 +641,7 @@ def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sa
         headers=[('Content-Type', 'application/json'), auth_header])
     assert response.status_code == 400
     resp_json = json.loads(response.get_data(as_text=True))
-    assert 'email_reply_to_id {} does not exist in database for service id {}'.\
+    assert 'email_reply_to_id {} does not exist in database for service id {}'. \
         format(fake_uuid, sample_email_template.service_id) in resp_json['errors'][0]['message']
     assert 'BadRequestError' in resp_json['errors'][0]['error']
 
@@ -637,6 +670,7 @@ def test_post_email_notification_with_valid_reply_to_id_returns_201(client, samp
 
     assert email_reply_to.notification_id == notification.id
     assert email_reply_to.service_email_reply_to_id == reply_to_email.id
+    assert notification.reply_to_text == reply_to_email.email_address
 
 
 def test_persist_sender_to_notification_mapping_for_email(notify_db_session):