From a98e5247b8d153611607824068ae4711bfa36421 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Fri, 15 Dec 2017 17:13:55 +0000 Subject: [PATCH] Update notificaiton API endpoints to use template's reply_to Sets the reply_to_text on notification from the template value if it's set. --- app/service/send_notification.py | 27 +++++++++---------- app/v2/notifications/post_notifications.py | 18 ++++++------- .../service/test_send_one_off_notification.py | 23 +++++++++++++++- .../notifications/test_post_notifications.py | 24 +++++++++++++++++ 4 files changed, 67 insertions(+), 25 deletions(-) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 634edc5ff..53cbb2b6c 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -15,7 +15,6 @@ from app.models import ( PRIORITY, SMS_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 @@ -56,7 +55,12 @@ 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) + reply_to = get_reply_to_text( + notification_type=template.template_type, + sender_id=sender_id, + service=service, + template=template + ) notification = persist_notification( template_id=template.id, template_version=template.version, @@ -80,21 +84,14 @@ def send_one_off_notification(service_id, post_data): return {'id': str(notification.id)} -def get_reply_to_text(notification_type, sender_id, service): +def get_reply_to_text(notification_type, sender_id, service, template): reply_to = None - if notification_type == EMAIL_TYPE: - if sender_id: + if sender_id: + if notification_type == EMAIL_TYPE: 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: + elif notification_type == SMS_TYPE: 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() + else: + reply_to = template.get_reply_to_text() return reply_to diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index ed9d67c7f..5466be234 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -70,8 +70,6 @@ def post_notification(notification_type): check_rate_limiting(authenticated_service, api_user) - reply_to = get_reply_to_text(notification_type, form) - template, template_with_content = validate_template( form['template_id'], form.get('personalisation', {}), @@ -79,11 +77,14 @@ def post_notification(notification_type): notification_type, ) + reply_to = get_reply_to_text(notification_type, form, template) + if notification_type == LETTER_TYPE: notification = process_letter_notification( letter_data=form, api_key=api_user, template=template, + reply_to_text=reply_to ) else: notification = process_sms_or_email_notification( @@ -164,7 +165,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ return notification -def process_letter_notification(*, letter_data, api_key, template): +def process_letter_notification(*, letter_data, api_key, template, reply_to_text): if api_key.key_type == KEY_TYPE_TEAM: raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) @@ -175,12 +176,11 @@ 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=letter_data, template=template, api_key=api_key, status=status, - reply_to_text=letter_contact_block) + reply_to_text=reply_to_text) if not should_send: update_letter_notifications_to_sent_to_dvla.apply_async( @@ -198,21 +198,21 @@ def process_letter_notification(*, letter_data, api_key, template): @statsd(namespace="performance-testing") -def get_reply_to_text(notification_type, form): +def get_reply_to_text(notification_type, form, template): 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() + ) or template.get_reply_to_text() 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() + ) or template.get_reply_to_text() elif notification_type == LETTER_TYPE: - reply_to = authenticated_service.get_default_letter_contact() + reply_to = template.get_reply_to_text() return reply_to diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 6a9a681ff..e64102ec1 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -18,6 +18,7 @@ from app.models import ( from tests.app.db import ( create_user, create_reply_to_email, + create_letter_contact, create_service, create_template ) @@ -217,7 +218,27 @@ def test_send_one_off_notification_should_add_email_reply_to_text_for_notificati research_mode=False, queue=None ) - notification.reply_to_text == reply_to_email.email_address + assert notification.reply_to_text == reply_to_email.email_address + + +def test_send_one_off_letter_notification_should_use_template_reply_to_text(sample_letter_template, celery_mock): + letter_contact = create_letter_contact(sample_letter_template.service, "Edinburgh, ED1 1AA", is_default=False) + sample_letter_template.reply_to = str(letter_contact.id) + + data = { + 'to': 'user@example.com', + 'template_id': str(sample_letter_template.id), + 'created_by': str(sample_letter_template.service.created_by_id) + } + + notification_id = send_one_off_notification(service_id=sample_letter_template.service.id, post_data=data) + notification = Notification.query.get(notification_id['id']) + celery_mock.assert_called_once_with( + notification=notification, + research_mode=False, + queue=None + ) + assert notification.reply_to_text == "Edinburgh, ED1 1AA" def test_send_one_off_notification_should_throw_exception_if_reply_to_id_doesnot_exist( diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index aa1bb70f6..cba00ff9b 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -28,6 +28,7 @@ from tests.app.db import ( create_service, create_template, create_reply_to_email, + create_letter_contact, create_service_sms_sender, create_service_with_inbound_number ) @@ -639,3 +640,26 @@ def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sa 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'] + + +def test_letter_notification_should_use_template_reply_to(client, sample_letter_template): + letter_contact = create_letter_contact(sample_letter_template.service, "Edinburgh, ED1 1AA", is_default=False) + sample_letter_template.reply_to = str(letter_contact.id) + + data = { + 'template_id': str(sample_letter_template.id), + 'personalisation': { + 'address_line_1': '123', + 'address_line_2': '234', + 'postcode': 'W1A1AA', + } + } + response = client.post("v2/notifications/letter", + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), + create_authorization_header(service_id=sample_letter_template.service.id)] + ) + assert response.status_code == 201, response.get_data(as_text=True) + notifications = Notification.query.all() + assert len(notifications) == 1 + assert notifications[0].reply_to_text == "Edinburgh, ED1 1AA"