diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 973aafaf5..4f05977ce 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -137,9 +137,9 @@ def validate_template(template_id, personalisation, service, notification_type): def check_service_email_reply_to_id(service_id, reply_to_id, notification_type): - if not (reply_to_id is None): + if reply_to_id: if notification_type != EMAIL_TYPE: - message = 'You sent a email_reply_to_id for a {} notification type'.format(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) @@ -150,9 +150,9 @@ 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 not (sms_sender_id is None): + if sms_sender_id: if notification_type != SMS_TYPE: - message = 'You sent a sms_sender_id for a {} notification type'.format(notification_type) + message = 'sms_sender_id is not a valid option for {} notification'.format(notification_type) raise BadRequestError(message=message) try: dao_get_service_sms_senders_by_id(service_id, sms_sender_id) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index cfbc2dd77..eb8bac3a5 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -12,9 +12,11 @@ from app.models import ( Notification, NotificationEmailReplyTo, NotificationHistory, + NotificationSmsSender, NotificationStatistics, ScheduledNotification, EMAIL_TYPE, + SMS_TYPE, NOTIFICATION_STATUS_TYPES, NOTIFICATION_STATUS_TYPES_FAILED, NOTIFICATION_SENT, @@ -22,16 +24,20 @@ from app.models import ( KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - JOB_STATUS_IN_PROGRESS, NotificationSmsSender, SMS_TYPE) + JOB_STATUS_IN_PROGRESS +) from app.dao.notifications_dao import ( dao_create_notification, dao_create_notification_email_reply_to_mapping, + dao_create_notification_sms_sender_mapping, dao_created_scheduled_notification, dao_delete_notifications_and_history_by_id, + dao_get_last_notification_added_for_job_id, dao_get_last_template_usage, dao_get_notification_email_reply_for_notification, dao_get_notifications_by_to_field, + dao_get_notification_sms_sender_mapping, dao_get_notification_statistics_for_service_and_day, dao_get_potential_notification_statistics_for_day, dao_get_scheduled_notifications, @@ -39,6 +45,7 @@ from app.dao.notifications_dao import ( dao_timeout_notifications, dao_update_notification, dao_update_notifications_for_job_to_sent_to_dvla, + dao_update_notifications_by_reference, delete_notifications_created_more_than_a_week_ago_by_type, get_notification_by_id, get_notification_for_job, @@ -49,11 +56,8 @@ from app.dao.notifications_dao import ( is_delivery_slow_for_provider, set_scheduled_notification_to_processed, update_notification_status_by_id, - update_notification_status_by_reference, - dao_get_last_notification_added_for_job_id, - dao_update_notifications_by_reference, - dao_create_notification_sms_sender_mapping, - dao_get_notification_sms_sender_mapping) + update_notification_status_by_reference +) from app.dao.services_dao import dao_update_service from tests.app.db import ( diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 941ed14f0..ff9521906 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -8,15 +8,23 @@ from freezegun import freeze_time from collections import namedtuple 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, \ - NotificationSmsSender +from app.models import ( + Notification, + NotificationHistory, + NotificationEmailReplyTo, + NotificationSmsSender, + ScheduledNotification, + Template +) from app.notifications.process_notifications import ( create_content_for_notification, persist_notification, - send_notification_to_queue, - simulated_recipient, + persist_email_reply_to_id_for_notification, persist_scheduled_notification, - persist_email_reply_to_id_for_notification, persist_sms_sender_id_for_notification) + persist_sms_sender_id_for_notification, + send_notification_to_queue, + simulated_recipient +) 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 diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index d9c643620..ede6d940d 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -311,9 +311,9 @@ def test_should_not_rate_limit_if_limiting_is_disabled( @pytest.mark.parametrize('key_type', ['test', 'normal']) def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms( - key_type, - notify_db, - notify_db_session, + key_type, + notify_db, + notify_db_session, ): service = create_service(notify_db, notify_db_session, permissions=[SMS_TYPE]) with pytest.raises(BadRequestError) as e: @@ -336,12 +336,17 @@ def test_check_service_email_reply_to_id_where_reply_to_id_is_none(notification_ assert check_service_email_reply_to_id(None, None, notification_type) is None +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 + + 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, reply_to_address.id, EMAIL_TYPE) assert e.value.status_code == 400 - assert e.value.message == 'email_reply_to_id {} does not exist in database for service id {}'\ + assert e.value.message == 'email_reply_to_id {} does not exist in database for service id {}' \ .format(reply_to_address.id, fake_uuid) @@ -349,30 +354,25 @@ def test_check_service_email_reply_to_id_where_reply_to_id_is_not_found(sample_s with pytest.raises(BadRequestError) as e: check_service_email_reply_to_id(sample_service.id, fake_uuid, EMAIL_TYPE) assert e.value.status_code == 400 - assert e.value.message == 'email_reply_to_id {} does not exist in database for service id {}'\ + 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_sms_sender_id_where_sms_sender_id_is_found(sample_service): - sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456') - assert check_service_sms_sender_id(sample_service.id, sms_sender.id, SMS_TYPE) is None - - -@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') +@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_sms_sender_id(sample_service.id, sms_sender.id, notification_type) + 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 == 'You sent a sms_sender_id for a {} notification type'.format(notification_type) + 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_reply_to_id_is_valid(notification_type): +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 -def test_check_service_sms_sender_id_where_reply_to_id_is_found(sample_service): +def test_check_service_sms_sender_id_where_sms_sender_id_is_found(sample_service): sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456') assert check_service_sms_sender_id(sample_service.id, sms_sender.id, SMS_TYPE) is None @@ -382,22 +382,22 @@ def test_check_service_sms_sender_id_where_service_id_is_not_found(sample_servic with pytest.raises(BadRequestError) as e: check_service_sms_sender_id(fake_uuid, sms_sender.id, SMS_TYPE) assert e.value.status_code == 400 - assert e.value.message == 'sms_sender_id {} does not exist in database for service id {}'\ + assert e.value.message == 'sms_sender_id {} does not exist in database for service id {}' \ .format(sms_sender.id, fake_uuid) -def test_check_service_sms_sender_id_where_reply_to_id_is_not_found(sample_service, fake_uuid): +def test_check_service_sms_sender_id_where_sms_sender_is_not_found(sample_service, fake_uuid): with pytest.raises(BadRequestError) as e: check_service_sms_sender_id(sample_service.id, fake_uuid, SMS_TYPE) assert e.value.status_code == 400 - assert e.value.message == 'sms_sender_id {} does not exist in database for service id {}'\ + 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_email_reply_to_id_when_channel_type_is_wrong(sample_service, notification_type): +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 == 'You sent a sms_sender_id for a {} notification type'.format(notification_type) + assert e.value.message == 'sms_sender_id is not a valid option for {} notification'.format(notification_type) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index a76702706..d4055df3f 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -16,6 +16,7 @@ from app.models import Notification from app.schema_validation import validate from app.v2.errors import RateLimitError from app.v2.notifications.notification_schemas import post_sms_response, post_email_response +from app.v2.notifications.post_notifications import persist_sender_to_notification_mapping from tests import create_authorization_header from tests.app.conftest import ( sample_template as create_sample_template, @@ -28,7 +29,7 @@ from tests.app.db import ( create_service, create_template, create_reply_to_email, - create_service_sms_sender) + create_service_sms_sender, create_notification) @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -561,7 +562,7 @@ 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 'You sent a {} for a {} notification type'.\ + assert '{} is not a valid option for {} notification'.\ format(form_label, notification_type) in resp_json['errors'][0]['message'] assert 'BadRequestError' in resp_json['errors'][0]['error'] @@ -630,3 +631,37 @@ 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 + + +def test_persist_sender_to_notification_mapping_for_email(notify_db_session): + service = create_service() + template = create_template(service=service, template_type=EMAIL_TYPE) + sender = create_reply_to_email(service=service, email_address='reply@test.com', is_default=False) + form = { + "email_address": "recipient@test.com", + "template_id": str(template.id), + 'email_reply_to_id': str(sender.id) + } + notification = create_notification(template=template) + persist_sender_to_notification_mapping(form=form, notification=notification) + notification_to_email_reply_to = NotificationEmailReplyTo.query.all() + assert len(notification_to_email_reply_to) == 1 + assert notification_to_email_reply_to[0].notification_id == notification.id + assert notification_to_email_reply_to[0].service_email_reply_to_id == sender.id + + +def test_persist_sender_to_notification_mapping_for_sms(notify_db_session): + service = create_service() + template = create_template(service=service, template_type=SMS_TYPE) + sender = create_service_sms_sender(service=service, sms_sender='12345', is_default=False) + form = { + 'phone_number': '+447700900855', + 'template_id': str(template.id), + 'sms_sender_id': str(sender.id) + } + notification = create_notification(template=template) + persist_sender_to_notification_mapping(form=form, notification=notification) + notification_to_sms_sender = NotificationSmsSender.query.all() + assert len(notification_to_sms_sender) == 1 + assert notification_to_sms_sender[0].notification_id == notification.id + assert notification_to_sms_sender[0].service_sms_sender_id == sender.id