[WIP]

app/models.py

@@ -978,6 +978,8 @@ class Notification(db.Model):
     created_by = db.relationship('User')
     created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=True)
 
+    reply_to_text = db.Column(db.String, nullable=True)
+
     __table_args__ = (
         db.ForeignKeyConstraint(
             ['template_id', 'template_version'],

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/validators.py

@@ -142,7 +142,7 @@ def check_service_email_reply_to_id(service_id, reply_to_id, notification_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)

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)
-    sms_sender = check_service_sms_sender_id(str(authenticated_service.id), service_sms_sender_id, notification_type)
+    reply_to = get_reply_to_text(notification_type, form)
 
     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)
@@ -194,3 +193,20 @@ def process_letter_notification(*, letter_data, api_key, template):
         )
 
     return notification
+
+
+def get_reply_to_text(notification_type, form):
+    service_email_reply_to_id = form.get("email_reply_to_id", None)
+    service_sms_sender_id = form.get("sms_sender_id", None)
+
+    reply_to = check_service_email_reply_to_id(
+            str(authenticated_service.id), service_email_reply_to_id, notification_type
+        ) 
+    reply_to = check_service_sms_sender_id(str(authenticated_service.id), service_sms_sender_id, notification_type)
+    
+    if not reply_to:
+        if notification_type == EMAIL_TYPE:
+            reply_to = authenticated_service.get_default_reply_to_email_address()
+        elif notification_type == SMS_TYPE:
+            reply_to = authenticated_service.get_default_sms_sender()
+    return reply_to

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

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")])
@@ -225,6 +256,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 \
@@ -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
 
@@ -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):