diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 55665a642..ecdf314d2 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -40,7 +40,9 @@ from app.models import ( KEY_TYPE_NORMAL, KEY_TYPE_TEST, LETTER_TYPE, NOTIFICATION_SENT, - NotificationEmailReplyTo, ServiceEmailReplyTo) + NotificationEmailReplyTo, + ServiceEmailReplyTo +) from app.dao.dao_utils import transactional from app.statsd_decorators import statsd @@ -619,7 +621,7 @@ def dao_get_notification_email_reply_for_notification(notification_id): NotificationEmailReplyTo ).filter( NotificationEmailReplyTo.notification_id == notification_id - ).all() + ).first() - if len(email_reply_to) == 1: - return email_reply_to[0].email_address + if email_reply_to: + return email_reply_to.email_address diff --git a/app/notifications/validators.py b/app/notifications/validators.py index a76e70089..fe9f336a7 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -140,5 +140,6 @@ def check_service_email_reply_to_id(service_id, reply_to_id): try: reply_to = dao_get_reply_to_by_id(service_id, reply_to_id) except NoResultFound: - message = 'email_reply_to_id does not exist in database' + message = 'email_reply_to_id {} does not exist in database for service id {}'\ + .format(reply_to_id, service_id) raise BadRequestError(message=message) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 9e22e2346..43b277294 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -19,7 +19,9 @@ from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, simulated_recipient, - persist_scheduled_notification, persist_email_reply_to_id_for_notification) + persist_scheduled_notification, + persist_email_reply_to_id_for_notification +) from app.notifications.process_letter_notifications import ( create_letter_notification ) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 901eeb783..3124b3a13 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -11,6 +11,7 @@ from app.models import ( Notification, NotificationHistory, Job, + NotificationEmailReplyTo, NotificationStatistics, ScheduledNotification, NOTIFICATION_STATUS_TYPES, @@ -20,15 +21,23 @@ from app.models import ( KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - NotificationEmailReplyTo) +) from app.dao.notifications_dao import ( dao_create_notification, + dao_create_notification_email_reply_to_mapping, + dao_created_scheduled_notification, + dao_delete_notifications_and_history_by_id, + dao_get_notifications_by_to_field, dao_get_last_template_usage, + dao_get_notification_email_reply_for_notification, dao_get_notification_statistics_for_service_and_day, dao_get_potential_notification_statistics_for_day, + dao_get_scheduled_notifications, dao_get_template_usage, + dao_timeout_notifications, dao_update_notification, + dao_update_notifications_for_job_to_sent_to_dvla, delete_notifications_created_more_than_a_week_ago_by_type, get_notification_by_id, get_notification_for_job, @@ -36,17 +45,11 @@ from app.dao.notifications_dao import ( get_notifications_for_job, get_notifications_for_service, get_total_sent_notifications_in_date_range, - update_notification_status_by_id, - update_notification_status_by_reference, - dao_delete_notifications_and_history_by_id, - dao_timeout_notifications, is_delivery_slow_for_provider, - dao_update_notifications_for_job_to_sent_to_dvla, - dao_get_notifications_by_to_field, - dao_created_scheduled_notification, - dao_get_scheduled_notifications, set_scheduled_notification_to_processed, - dao_create_notification_email_reply_to_mapping, dao_get_notification_email_reply_for_notification) + update_notification_status_by_id, + update_notification_status_by_reference +) from app.dao.services_dao import dao_update_service from tests.app.db import create_notification, create_api_key, create_reply_to_email @@ -1968,15 +1971,12 @@ def test_dao_create_notification_email_reply_to_mapping(sample_service, sample_n def test_dao_create_multiple_notification_email_reply_to_mapping(sample_service, sample_notification): + reply_to_address = create_reply_to_email(sample_service, "test@test.com") - 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) + dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address.id) with pytest.raises(IntegrityError) as e: - dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address[0].id) + dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address.id) assert 'duplicate key value' in str(e.value) @@ -1984,15 +1984,14 @@ def test_dao_create_multiple_notification_email_reply_to_mapping(sample_service, 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 + assert email_reply_to[0].service_email_reply_to_id == reply_to_address.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) +def test_dao_get_notification_ememail_reply_toail_reply_for_notification(sample_service, sample_notification): + reply_to_address = create_reply_to_email(sample_service, "test@test.com") + dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address.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): +def test_dao_get_notification_email_reply_for_notification_where_no_mapping(notify_db_session, fake_uuid): assert dao_get_notification_email_reply_for_notification(fake_uuid) is None diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index b59decc9f..387da140e 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -330,43 +330,23 @@ 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): + reply_to_address = create_reply_to_email(sample_service, "test@test.com") with pytest.raises(BadRequestError) as e: - check_service_email_reply_to_id(fake_uuid, fake_uuid) + check_service_email_reply_to_id(fake_uuid, reply_to_address.id) 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 + assert e.value.message == 'email_reply_to_id {} does not exist in database for service id {}'\ + .format(reply_to_address.id, fake_uuid) 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' + assert e.value.message == 'email_reply_to_id {} does not exist in database for service id {}'\ + .format(fake_uuid, sample_service.id) 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 1f0c8091d..41c23b258 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -4,9 +4,13 @@ import pytest from freezegun import freeze_time from app.models import ( - Notification, ScheduledNotification, SCHEDULE_NOTIFICATIONS, - EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE, - NotificationEmailReplyTo) + NotificationEmailReplyTo, + ScheduledNotification, + SCHEDULE_NOTIFICATIONS, + EMAIL_TYPE, + INTERNATIONAL_SMS_TYPE, + SMS_TYPE +) from flask import json, current_app from app.models import Notification @@ -15,8 +19,10 @@ from app.v2.errors import RateLimitError from app.v2.notifications.notification_schemas import post_sms_response, post_email_response from tests import create_authorization_header from tests.app.conftest import ( - sample_template as create_sample_template, sample_service, - sample_template_without_sms_permission, sample_template_without_email_permission + sample_template as create_sample_template, + sample_service, + sample_template_without_email_permission, + sample_template_without_sms_permission ) from tests.app.db import create_inbound_number, create_service, create_template, create_reply_to_email @@ -501,7 +507,8 @@ def test_post_sms_notification_with_invalid_reply_to_email_id( 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 'reply_to_id {} does not exist in database for service id {}'.\ + format(fake_uuid, sample_template_with_placeholders.service_id) in resp_json['errors'][0]['message'] assert 'BadRequestError' in resp_json['errors'][0]['error'] @@ -540,7 +547,8 @@ 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 'reply_to_id does not exist in database' in resp_json['errors'][0]['message'] + assert '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']