From d2168b798580f1f70d4bff6a5ab28976bd71e40b Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 5 Oct 2017 11:33:20 +0100 Subject: [PATCH] Added the mapping between notification and reply to email to the database and persisted the mapping when the request is received by the end point. the end point also checks if the reply to email id exists and if not returns an error. Also added tests to test the functionality. --- app/dao/notifications_dao.py | 22 ++++++- app/notifications/process_notifications.py | 7 ++- app/v2/notifications/post_notifications.py | 6 +- .../notification_dao/test_notification_dao.py | 57 +++++++++++++++++-- .../test_process_notification.py | 20 ++++++- tests/app/notifications/test_validators.py | 23 ++++++++ .../notifications/test_post_notifications.py | 49 +++++++++++++++- 7 files changed, 171 insertions(+), 13 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index fef7b9567..55665a642 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -40,7 +40,7 @@ from app.models import ( KEY_TYPE_NORMAL, KEY_TYPE_TEST, LETTER_TYPE, NOTIFICATION_SENT, -) + NotificationEmailReplyTo, ServiceEmailReplyTo) from app.dao.dao_utils import transactional from app.statsd_decorators import statsd @@ -603,3 +603,23 @@ def dao_set_created_live_letter_api_notifications_to_pending(): db.session.commit() return notifications + + +@transactional +def dao_create_notification_email_reply_to_mapping(notification_id, email_reply_to_id): + notification_email_reply_to = NotificationEmailReplyTo( + notification_id=notification_id, + service_email_reply_to_id=email_reply_to_id + ) + db.session.add(notification_email_reply_to) + + +def dao_get_notification_email_reply_for_notification(notification_id): + email_reply_to = ServiceEmailReplyTo.query.join( + NotificationEmailReplyTo + ).filter( + NotificationEmailReplyTo.notification_id == notification_id + ).all() + + if len(email_reply_to) == 1: + return email_reply_to[0].email_address diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 8a75473b4..2b8992ba1 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -16,7 +16,8 @@ from app.config import QueueNames from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, NOTIFICATION_CREATED, ScheduledNotification from app.dao.notifications_dao import (dao_create_notification, dao_delete_notifications_and_history_by_id, - dao_created_scheduled_notification) + dao_created_scheduled_notification, + dao_create_notification_email_reply_to_mapping) from app.v2.errors import BadRequestError from app.utils import get_template_instance, cache_key_for_service_template_counter, convert_bst_to_utc @@ -141,3 +142,7 @@ def persist_scheduled_notification(notification_id, scheduled_for): scheduled_notification = ScheduledNotification(notification_id=notification_id, scheduled_for=scheduled_datetime) dao_created_scheduled_notification(scheduled_notification) + + +def persist_email_reply_to_id_for_notification(notification_id, email_reply_to_id): + dao_create_notification_email_reply_to_mapping(notification_id, email_reply_to_id) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 04c63a3e7..9e22e2346 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -19,7 +19,7 @@ from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, simulated_recipient, - persist_scheduled_notification) + persist_scheduled_notification, persist_email_reply_to_id_for_notification) from app.notifications.process_letter_notifications import ( create_letter_notification ) @@ -140,6 +140,10 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ simulated=simulated ) + email_reply_to_id = form.get("email_reply_to_id", None) + if email_reply_to_id is not None: + persist_email_reply_to_id_for_notification(notification.id, email_reply_to_id) + scheduled_for = form.get("scheduled_for", None) if scheduled_for: persist_scheduled_notification(notification.id, form["scheduled_for"]) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index ba8378827..901eeb783 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -6,6 +6,7 @@ import pytest from freezegun import freeze_time from sqlalchemy.exc import SQLAlchemyError, IntegrityError +from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_service_id from app.models import ( Notification, NotificationHistory, @@ -18,8 +19,8 @@ from app.models import ( NOTIFICATION_DELIVERED, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, - KEY_TYPE_TEST -) + KEY_TYPE_TEST, + NotificationEmailReplyTo) from app.dao.notifications_dao import ( dao_create_notification, @@ -44,11 +45,11 @@ from app.dao.notifications_dao import ( dao_get_notifications_by_to_field, dao_created_scheduled_notification, dao_get_scheduled_notifications, - set_scheduled_notification_to_processed -) + set_scheduled_notification_to_processed, + dao_create_notification_email_reply_to_mapping, dao_get_notification_email_reply_for_notification) from app.dao.services_dao import dao_update_service -from tests.app.db import create_notification, create_api_key +from tests.app.db import create_notification, create_api_key, create_reply_to_email from tests.app.conftest import ( sample_notification, sample_template, @@ -1949,3 +1950,49 @@ def test_dao_get_notifications_by_to_field_orders_by_created_at_desc(sample_temp assert len(notifications) == 2 assert notifications[0].id == notification.id assert notifications[1].id == notification_a_minute_ago.id + + +def test_dao_create_notification_email_reply_to_mapping(sample_service, sample_notification): + + create_reply_to_email(sample_service, "test@test.com") + + reply_to_address = dao_get_reply_to_by_service_id(sample_service.id) + + dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address[0].id) + + email_reply_to = NotificationEmailReplyTo.query.all() + + assert len(email_reply_to) == 1 + assert email_reply_to[0].notification_id == sample_notification.id + assert email_reply_to[0].service_email_reply_to_id == reply_to_address[0].id + + +def test_dao_create_multiple_notification_email_reply_to_mapping(sample_service, sample_notification): + + create_reply_to_email(sample_service, "test@test.com") + + reply_to_address = dao_get_reply_to_by_service_id(sample_service.id) + + dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address[0].id) + + with pytest.raises(IntegrityError) as e: + dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address[0].id) + + assert 'duplicate key value' in str(e.value) + + email_reply_to = NotificationEmailReplyTo.query.all() + + assert len(email_reply_to) == 1 + assert email_reply_to[0].notification_id == sample_notification.id + assert email_reply_to[0].service_email_reply_to_id == reply_to_address[0].id + + +def test_dao_get_notification_email_reply_for_notification(sample_service, sample_notification): + create_reply_to_email(sample_service, "test@test.com") + reply_to_address = dao_get_reply_to_by_service_id(sample_service.id) + dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address[0].id) + assert dao_get_notification_email_reply_for_notification(sample_notification.id) == "test@test.com" + + +def test_dao_get_notification_email_reply_for_notification_where_no_mapping(fake_uuid): + assert dao_get_notification_email_reply_for_notification(fake_uuid) is None diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index fbe9787c6..e8e67568b 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -7,18 +7,20 @@ from sqlalchemy.exc import SQLAlchemyError from freezegun import freeze_time from collections import namedtuple -from app.models import Template, Notification, NotificationHistory, ScheduledNotification +from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_service_id +from app.models import Template, Notification, NotificationHistory, ScheduledNotification, NotificationEmailReplyTo from app.notifications.process_notifications import ( create_content_for_notification, persist_notification, send_notification_to_queue, simulated_recipient, - persist_scheduled_notification -) + persist_scheduled_notification, + persist_email_reply_to_id_for_notification) from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address from app.utils import cache_key_for_service_template_counter from app.v2.errors import BadRequestError from tests.app.conftest import sample_api_key as create_api_key +from tests.app.db import create_reply_to_email def test_create_content_for_notification_passes(sample_email_template): @@ -438,3 +440,15 @@ def test_persist_email_notification_stores_normalised_email( assert persisted_notification.to == recipient assert persisted_notification.normalised_to == expected_recipient_normalised + + +def test_persist_email_reply_to_id_for_notification(sample_service, sample_notification): + create_reply_to_email(sample_service, "test@test.com") + reply_to_address = dao_get_reply_to_by_service_id(sample_service.id) + persist_email_reply_to_id_for_notification(sample_notification.id, reply_to_address[0].id) + + email_reply_to = NotificationEmailReplyTo.query.all() + + assert len(email_reply_to) == 1 + assert email_reply_to[0].notification_id == sample_notification.id + assert email_reply_to[0].service_email_reply_to_id == reply_to_address[0].id diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 66779b70e..b59decc9f 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -347,3 +347,26 @@ def test_check_service_email_reply_to_id_where_service_id_is_not_found(sample_se check_service_email_reply_to_id(fake_uuid, fake_uuid) assert e.value.status_code == 400 assert e.value.message == 'email_reply_to_id does not exist in database' + + +def test_check_service_email_reply_to_id_where_reply_to_id_is_none(): + assert check_service_email_reply_to_id(None, None) is None + + +def test_check_service_email_reply_to_id_where_reply_to_id_is_not_found(sample_service, fake_uuid): + with pytest.raises(BadRequestError) as e: + check_service_email_reply_to_id(sample_service.id, fake_uuid) + assert e.value.status_code == 400 + assert e.value.message == 'email_reply_to_id does not exist in database' + + +def test_check_service_email_reply_to_id_where_reply_to_id_is_found(sample_service): + reply_to_email = create_reply_to_email(sample_service, 'test@test.com') + assert check_service_email_reply_to_id(sample_service.id, reply_to_email.id) is None + + +def test_check_service_email_reply_to_id_where_service_id_is_not_found(sample_service, fake_uuid): + with pytest.raises(BadRequestError) as e: + check_service_email_reply_to_id(fake_uuid, fake_uuid) + assert e.value.status_code == 400 + assert e.value.message == 'email_reply_to_id does not exist in database' diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index d4fdf94ec..1f0c8091d 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -5,8 +5,8 @@ from freezegun import freeze_time from app.models import ( Notification, ScheduledNotification, SCHEDULE_NOTIFICATIONS, - EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE -) + EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE, + NotificationEmailReplyTo) from flask import json, current_app from app.models import Notification @@ -524,3 +524,48 @@ def test_post_email_notification_with_valid_reply_to_id_returns_201(client, samp notification = Notification.query.first() assert resp_json['id'] == str(notification.id) assert mocked.called + + +def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sample_email_template, mocker, fake_uuid): + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = { + "email_address": sample_email_template.service.users[0].email_address, + "template_id": sample_email_template.id, + 'email_reply_to_id': fake_uuid + } + auth_header = create_authorization_header(service_id=sample_email_template.service_id) + response = client.post( + path="v2/notifications/email", + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 400 + resp_json = json.loads(response.get_data(as_text=True)) + assert 'reply_to_id does not exist in database' in resp_json['errors'][0]['message'] + assert 'BadRequestError' in resp_json['errors'][0]['error'] + + +def test_post_email_notification_with_valid_reply_to_id_returns_201(client, sample_email_template, mocker): + reply_to_email = create_reply_to_email(sample_email_template.service, 'test@test.com') + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = { + "email_address": sample_email_template.service.users[0].email_address, + "template_id": sample_email_template.id, + 'email_reply_to_id': reply_to_email.id + } + auth_header = create_authorization_header(service_id=sample_email_template.service_id) + response = client.post( + path="v2/notifications/email", + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + assert validate(resp_json, post_email_response) == resp_json + notification = Notification.query.first() + assert resp_json['id'] == str(notification.id) + assert mocked.called + + email_reply_to = NotificationEmailReplyTo.query.all() + + assert len(email_reply_to) == 1 + assert email_reply_to[0].notification_id == notification.id + assert email_reply_to[0].service_email_reply_to_id == reply_to_email.id