diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 1c010c86b..59263cd9f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -33,6 +33,7 @@ from app.models import ( ServiceEmailReplyTo, Template, EMAIL_TYPE, + SMS_TYPE, KEY_TYPE_NORMAL, KEY_TYPE_TEST, LETTER_TYPE, @@ -43,7 +44,9 @@ from app.models import ( NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, - NOTIFICATION_SENT + NOTIFICATION_SENT, + NotificationSmsSender, + ServiceSmsSender ) from app.dao.dao_utils import transactional @@ -372,15 +375,19 @@ def delete_notifications_created_more_than_a_week_ago_by_type(notification_type) seven_days_ago = date.today() - timedelta(days=7) # Following could be refactored when NotificationSmsReplyTo and NotificationLetterContact in models.py - if notification_type == EMAIL_TYPE: + if notification_type in [EMAIL_TYPE, SMS_TYPE]: subq = db.session.query(Notification.id).filter( func.date(Notification.created_at) < seven_days_ago, Notification.notification_type == notification_type ).subquery() - deleted = db.session.query( - NotificationEmailReplyTo + if notification_type == EMAIL_TYPE: + notification_sender_mapping_table = NotificationEmailReplyTo + if notification_type == SMS_TYPE: + notification_sender_mapping_table = NotificationSmsSender + db.session.query( + notification_sender_mapping_table ).filter( - NotificationEmailReplyTo.notification_id.in_(subq) + notification_sender_mapping_table.notification_id.in_(subq) ).delete(synchronize_session='fetch') deleted = db.session.query(Notification).filter( @@ -650,3 +657,23 @@ def dao_get_last_notification_added_for_job_id(job_id): ).first() return last_notification_added + + +@transactional +def dao_create_notification_sms_sender_mapping(notification_id, sms_sender_id): + notification_to_sms_sender = NotificationSmsSender( + notification_id=notification_id, + service_sms_sender_id=sms_sender_id + ) + db.session.add(notification_to_sms_sender) + + +def dao_get_notification_sms_sender_mapping(notification_id): + sms_sender = ServiceSmsSender.query.join( + NotificationSmsSender + ).filter( + NotificationSmsSender.notification_id == notification_id + ).first() + + if sms_sender: + return sms_sender.sms_sender diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index e0a418230..b22c07e58 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -10,7 +10,7 @@ from app.dao.dao_utils import ( transactional, version_class ) -from app.dao.notifications_dao import get_financial_year +from app.dao.date_util import get_financial_year from app.dao.service_sms_sender_dao import insert_service_sms_sender from app.models import ( NotificationStatistics, diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index c253748ad..324c21582 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -11,8 +11,8 @@ from notifications_utils.template import HTMLEmailTemplate, PlainTextEmailTempla from app import clients, statsd_client, create_uuid from app.dao.notifications_dao import ( dao_update_notification, - dao_get_notification_email_reply_for_notification -) + dao_get_notification_email_reply_for_notification, + dao_get_notification_sms_sender_mapping) from app.dao.provider_details_dao import ( get_provider_details_by_notification_type, dao_toggle_sms_provider @@ -69,11 +69,15 @@ def send_sms_to_provider(notification): raise else: try: + sms_sender = dao_get_notification_sms_sender_mapping(notification.id) + if not sms_sender: + sms_sender = service.get_default_sms_sender() + provider.send_sms( to=validate_and_format_phone_number(notification.to, international=notification.international), content=str(template), reference=str(notification.id), - sender=service.get_default_sms_sender() + sender=sms_sender ) except Exception as e: dao_toggle_sms_provider(provider.name) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 21fda224e..76e18ebf4 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -17,7 +17,8 @@ from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, NOTIFI from app.dao.notifications_dao import (dao_create_notification, dao_delete_notifications_and_history_by_id, dao_created_scheduled_notification, - dao_create_notification_email_reply_to_mapping) + dao_create_notification_email_reply_to_mapping, + dao_create_notification_sms_sender_mapping) from app.v2.errors import BadRequestError from app.utils import get_template_instance, cache_key_for_service_template_counter, convert_bst_to_utc @@ -146,3 +147,7 @@ def persist_scheduled_notification(notification_id, scheduled_for): 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) + + +def persist_sms_sender_id_for_notification(notification_id, sms_sender_id): + dao_create_notification_sms_sender_mapping(notification_id, sms_sender_id) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 181d48b6e..4f05977ce 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -8,6 +8,7 @@ from notifications_utils.recipients import ( from notifications_utils.clients.redis import rate_limit_cache_key, daily_limit_cache_key from app.dao import services_dao, templates_dao +from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id from app.models import ( INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, KEY_TYPE_TEAM, SCHEDULE_NOTIFICATIONS @@ -135,11 +136,27 @@ def validate_template(template_id, personalisation, service, notification_type): return template, template_with_content -def check_service_email_reply_to_id(service_id, reply_to_id): - if not (reply_to_id is None): +def check_service_email_reply_to_id(service_id, reply_to_id, notification_type): + if reply_to_id: + if notification_type != EMAIL_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) except NoResultFound: message = 'email_reply_to_id {} does not exist in database for service id {}'\ .format(reply_to_id, service_id) raise BadRequestError(message=message) + + +def check_service_sms_sender_id(service_id, sms_sender_id, notification_type): + if sms_sender_id: + if notification_type != SMS_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) + except NoResultFound: + message = 'sms_sender_id {} does not exist in database for service id {}'\ + .format(sms_sender_id, service_id) + raise BadRequestError(message=message) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index d1ed9a5a3..fa6dd1bb2 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -2,13 +2,13 @@ from app.config import QueueNames from app.notifications.validators import ( check_service_over_daily_message_limit, validate_and_format_recipient, - validate_template, - check_service_email_reply_to_id) + validate_template) from app.notifications.process_notifications import ( - create_content_for_notification, persist_notification, send_notification_to_queue, - persist_email_reply_to_id_for_notification) + persist_email_reply_to_id_for_notification, + persist_sms_sender_id_for_notification +) from app.models import ( KEY_TYPE_NORMAL, PRIORITY, @@ -64,9 +64,11 @@ def send_one_off_notification(service_id, post_data): created_by_id=post_data['created_by'] ) sender_id = post_data.get('sender_id', None) - if sender_id and template.template_type == EMAIL_TYPE: - check_service_email_reply_to_id(service_id, sender_id) - persist_email_reply_to_id_for_notification(notification.id, sender_id) + if sender_id: + if template.template_type == EMAIL_TYPE: + persist_email_reply_to_id_for_notification(notification.id, sender_id) + if template.template_type == SMS_TYPE: + persist_sms_sender_id_for_notification(notification.id, sender_id) queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None send_notification_to_queue( diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 1689a7319..258b5df0d 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -119,7 +119,8 @@ post_sms_request = { "phone_number": {"type": "string", "format": "phone_number"}, "template_id": uuid, "personalisation": personalisation, - "scheduled_for": {"type": ["string", "null"], "format": "datetime"} + "scheduled_for": {"type": ["string", "null"], "format": "datetime"}, + "sms_sender_id": uuid }, "required": ["phone_number", "template_id"] } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index b4c837306..87d55159d 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -20,8 +20,8 @@ from app.notifications.process_notifications import ( persist_notification, persist_scheduled_notification, send_notification_to_queue, - simulated_recipient -) + simulated_recipient, + persist_sms_sender_id_for_notification) from app.notifications.process_letter_notifications import ( create_letter_notification ) @@ -31,7 +31,8 @@ from app.notifications.validators import ( check_service_can_schedule_notification, check_service_has_permission, validate_template, - check_service_email_reply_to_id + check_service_email_reply_to_id, + check_service_sms_sender_id ) from app.schema_validation import validate from app.v2.errors import BadRequestError @@ -63,12 +64,14 @@ def post_notification(notification_type): 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) + check_service_email_reply_to_id(str(authenticated_service.id), service_email_reply_to_id, notification_type) + check_service_sms_sender_id(str(authenticated_service.id), service_sms_sender_id, notification_type) template, template_with_content = validate_template( form['template_id'], @@ -142,9 +145,7 @@ 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: - persist_email_reply_to_id_for_notification(notification.id, email_reply_to_id) + persist_sender_to_notification_mapping(form, notification) scheduled_for = form.get("scheduled_for", None) if scheduled_for: @@ -163,6 +164,15 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ return notification +def persist_sender_to_notification_mapping(form, notification): + email_reply_to_id = form.get("email_reply_to_id", None) + if email_reply_to_id: + persist_email_reply_to_id_for_notification(notification.id, email_reply_to_id) + sms_sender_id = form.get("sms_sender_id", None) + if sms_sender_id: + persist_sms_sender_id_for_notification(notification.id, sms_sender_id) + + def process_letter_notification(*, letter_data, api_key, template): if api_key.key_type == KEY_TYPE_TEAM: raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 562fd8555..eb8bac3a5 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -12,10 +12,11 @@ from app.models import ( Notification, NotificationEmailReplyTo, NotificationHistory, + NotificationSmsSender, NotificationStatistics, ScheduledNotification, - ServiceEmailReplyTo, EMAIL_TYPE, + SMS_TYPE, NOTIFICATION_STATUS_TYPES, NOTIFICATION_STATUS_TYPES_FAILED, NOTIFICATION_SENT, @@ -23,16 +24,20 @@ from app.models import ( KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - JOB_STATUS_IN_PROGRESS) + 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, @@ -40,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, @@ -50,16 +56,16 @@ 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) + update_notification_status_by_reference +) from app.dao.services_dao import dao_update_service from tests.app.db import ( create_api_key, create_job, create_notification, - create_reply_to_email -) + create_reply_to_email, + create_service_sms_sender) from tests.app.conftest import ( sample_notification, sample_template, @@ -1042,6 +1048,38 @@ def test_should_delete_notification_to_email_reply_to_after_seven_days( assert notification.created_at.date() >= date(2016, 1, 3) +@freeze_time("2016-01-10 12:00:00.000000") +def test_should_delete_notification_to_sms_sender_after_seven_days( + sample_template +): + assert len(Notification.query.all()) == 0 + + sms_sender = create_service_sms_sender(service=sample_template.service, sms_sender='123456', is_default=False) + + # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type + for i in range(1, 11): + past_date = '2016-01-{0:02d} {0:02d}:00:00.000000'.format(i) + with freeze_time(past_date): + notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id) + + all_notifications = Notification.query.all() + assert len(all_notifications) == 10 + + all_notification_sms_senders = NotificationSmsSender.query.all() + assert len(all_notification_sms_senders) == 10 + + # Records before 3rd should be deleted + delete_notifications_created_more_than_a_week_ago_by_type(SMS_TYPE) + remaining_notifications = Notification.query.filter_by(notification_type=SMS_TYPE).all() + remaining_notification_to_sms_sender = NotificationSmsSender.query.filter_by().all() + + assert len(remaining_notifications) == 8 + assert len(remaining_notification_to_sms_sender) == 8 + + for notification in remaining_notifications: + assert notification.created_at.date() >= date(2016, 1, 3) + + @pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) @freeze_time("2016-01-10 12:00:00.000000") def test_should_not_delete_notification_history(notify_db, notify_db_session, sample_service, notification_type): @@ -2100,3 +2138,26 @@ def test_dao_update_notifications_by_reference_returns_zero_when_no_notification "billable_units": 2} ) assert updated_count == 0 + + +def test_dao_create_notification_sms_sender_mapping(sample_notification): + sms_sender = create_service_sms_sender(service=sample_notification.service, sms_sender='123456') + dao_create_notification_sms_sender_mapping(notification_id=sample_notification.id, + sms_sender_id=sms_sender.id) + notification_to_senders = NotificationSmsSender.query.all() + assert len(notification_to_senders) == 1 + assert notification_to_senders[0].notification_id == sample_notification.id + assert notification_to_senders[0].service_sms_sender_id == sms_sender.id + + +def test_dao_get_notification_sms_sender_mapping(sample_notification): + sms_sender = create_service_sms_sender(service=sample_notification.service, sms_sender='123456') + dao_create_notification_sms_sender_mapping(notification_id=sample_notification.id, + sms_sender_id=sms_sender.id) + notification_to_sender = dao_get_notification_sms_sender_mapping(sample_notification.id) + assert notification_to_sender == '123456' + + +def test_dao_get_notification_sms_sender_mapping_returns_none(sample_notification): + notification_to_sender = dao_get_notification_sms_sender_mapping(sample_notification.id) + assert not notification_to_sender diff --git a/tests/app/db.py b/tests/app/db.py index 0cdf7cfee..4be3a3799 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -29,7 +29,11 @@ from app.models import ( KEY_TYPE_NORMAL ) from app.dao.users_dao import save_model_user -from app.dao.notifications_dao import dao_create_notification, dao_created_scheduled_notification +from app.dao.notifications_dao import ( + dao_create_notification, + dao_created_scheduled_notification, + dao_create_notification_sms_sender_mapping +) from app.dao.templates_dao import dao_create_template from app.dao.services_dao import dao_create_service from app.dao.service_permissions_dao import dao_add_service_permission @@ -128,6 +132,7 @@ def create_notification( scheduled_for=None, normalised_to=None, one_off=False, + sms_sender_id=None ): if created_at is None: created_at = datetime.utcnow() @@ -184,6 +189,9 @@ def create_notification( if status != 'created': scheduled_notification.pending = False dao_created_scheduled_notification(scheduled_notification) + if sms_sender_id: + dao_create_notification_sms_sender_mapping(notification_id=notification.id, + sms_sender_id=sms_sender_id) return notification diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 6733c5846..2d6a10cdf 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -32,8 +32,8 @@ from tests.app.db import ( create_notification, create_inbound_number, create_reply_to_email, - create_reply_to_email_for_notification -) + create_reply_to_email_for_notification, + create_service_sms_sender) def test_should_return_highest_priority_active_provider(restore_provider_details): @@ -343,6 +343,28 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): ) +def test_send_sms_should_use_service_sms_sender( + sample_service, + sample_template, + mocker): + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') + + sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456', is_default=False) + db_notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id) + + send_to_providers.send_sms_to_provider( + db_notification, + ) + + app.mmg_client.send_sms.assert_called_once_with( + to=ANY, + content=ANY, + reference=ANY, + sender=sms_sender.sms_sender + ) + + @pytest.mark.parametrize('research_mode,key_type', [ (True, KEY_TYPE_NORMAL), (False, KEY_TYPE_TEST) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index e8e67568b..ff9521906 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -8,19 +8,28 @@ 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 +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, + 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 from tests.app.conftest import sample_api_key as create_api_key -from tests.app.db import create_reply_to_email +from tests.app.db import create_reply_to_email, create_service_sms_sender def test_create_content_for_notification_passes(sample_email_template): @@ -452,3 +461,15 @@ def test_persist_email_reply_to_id_for_notification(sample_service, sample_notif 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_persist_sms_sender_id_for_notification(sample_service, sample_notification): + sms_sender = create_service_sms_sender(service=sample_service, sms_sender="123456") + persist_sms_sender_id_for_notification(notification_id=sample_notification.id, + sms_sender_id=sms_sender.id) + + notification_to_sms_sender = NotificationSmsSender.query.all() + + assert len(notification_to_sms_sender) == 1 + assert notification_to_sms_sender[0].notification_id == sample_notification.id + assert notification_to_sms_sender[0].service_sms_sender_id == sms_sender.id diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index fecfc4da5..ede6d940d 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -11,8 +11,8 @@ from app.notifications.validators import ( check_sms_content_char_count, check_service_over_api_rate_limit, validate_and_format_recipient, - check_service_email_reply_to_id -) + check_service_email_reply_to_id, + check_service_sms_sender_id) from app.v2.errors import ( BadRequestError, @@ -23,7 +23,7 @@ from tests.app.conftest import ( sample_service as create_service, sample_service_whitelist, sample_api_key) -from tests.app.db import create_reply_to_email +from tests.app.db import create_reply_to_email, create_service_sms_sender @pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) @@ -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: @@ -331,27 +331,73 @@ def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_s assert result == '201212341234' -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 +@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +def test_check_service_email_reply_to_id_where_reply_to_id_is_none(notification_type): + 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) + 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) 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) + 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_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 +@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_email_reply_to_id(sample_service.id, reply_to_address.id, notification_type) + assert e.value.status_code == 400 + 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_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_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 + + +def test_check_service_sms_sender_id_where_service_id_is_not_found(sample_service, fake_uuid): + sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456') + 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 {}' \ + .format(sms_sender.id, 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 {}' \ + .format(fake_uuid, sample_service.id) + + +@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') + 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 == 'sms_sender_id is not a valid option for {} notification'.format(notification_type) diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 1fe9dc155..64650434c 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -3,6 +3,7 @@ from unittest.mock import Mock import pytest from notifications_utils.recipients import InvalidPhoneError +from sqlalchemy.exc import SQLAlchemyError from app.v2.errors import BadRequestError, TooManyRequestsError from app.config import QueueNames @@ -12,9 +13,11 @@ from app.models import ( PRIORITY, SMS_TYPE, NotificationEmailReplyTo, - Notification) + Notification, + NotificationSmsSender +) -from tests.app.db import create_user, create_reply_to_email +from tests.app.db import create_user, create_reply_to_email, create_service_sms_sender @pytest.fixture @@ -206,7 +209,7 @@ def test_send_one_off_notification_should_add_email_reply_to_id_for_email(sample def test_send_one_off_notification_should_throw_exception_if_reply_to_id_doesnot_exist( - sample_email_template, celery_mock + sample_email_template ): data = { 'to': 'ok@ok.com', @@ -215,5 +218,39 @@ def test_send_one_off_notification_should_throw_exception_if_reply_to_id_doesnot 'created_by': str(sample_email_template.service.created_by_id) } - with pytest.raises(expected_exception=BadRequestError): + with pytest.raises(expected_exception=SQLAlchemyError): send_one_off_notification(service_id=sample_email_template.service.id, post_data=data) + + +def test_send_one_off_notification_should_add_sms_sender_mapping_for_sms(sample_template, celery_mock): + sms_sender = create_service_sms_sender(service=sample_template.service, sms_sender='123456') + data = { + 'to': '07700 900 001', + 'template_id': str(sample_template.id), + 'sender_id': sms_sender.id, + 'created_by': str(sample_template.service.created_by_id) + } + + notification_id = send_one_off_notification(service_id=sample_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 + ) + mapping_row = NotificationSmsSender.query.filter_by(notification_id=notification_id['id']).first() + assert mapping_row.service_sms_sender_id == sms_sender.id + + +def test_send_one_off_notification_should_throw_exception_if_sms_sender_id_doesnot_exist( + sample_template +): + data = { + 'to': '07700 900 001', + 'template_id': str(sample_template.id), + 'sender_id': str(uuid.uuid4()), + 'created_by': str(sample_template.service.created_by_id) + } + + with pytest.raises(expected_exception=SQLAlchemyError): + send_one_off_notification(service_id=sample_template.service.id, post_data=data) diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 8809a51bb..7062e7a30 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -28,7 +28,6 @@ from tests.app.db import ( create_service, create_inbound_number, create_reply_to_email, - create_service_sms_sender, create_letter_contact ) from tests.conftest import set_config diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 9d63c6bfb..d4055df3f 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -8,14 +8,15 @@ from app.models import ( ScheduledNotification, SCHEDULE_NOTIFICATIONS, EMAIL_TYPE, - SMS_TYPE -) + SMS_TYPE, + NotificationSmsSender) from flask import json, current_app 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, @@ -27,8 +28,8 @@ from tests.app.conftest import ( from tests.app.db import ( create_service, create_template, - create_reply_to_email -) + create_reply_to_email, + create_service_sms_sender, create_notification) @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -93,6 +94,34 @@ def test_post_sms_notification_uses_inbound_number_as_sender(client, notify_db_s 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 = { + 'phone_number': '+447700900855', + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'}, + 'sms_sender_id': str(sms_sender.id) + } + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + + response = client.post( + path='/v2/notifications/sms', + 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_sms_response) == resp_json + notification_to_sms_sender = NotificationSmsSender.query.all() + assert len(notification_to_sms_sender) == 1 + assert str(notification_to_sms_sender[0].notification_id) == resp_json['id'] + assert notification_to_sms_sender[0].service_sms_sender_id == sms_sender.id + mocked.assert_called_once_with([resp_json['id']], queue='send-sms-tasks') + + @pytest.mark.parametrize("notification_type, key_send_to, send_to", [("sms", "phone_number", "+447700900855"), ("email", "email_address", "sample@email.com")]) @@ -501,30 +530,40 @@ def test_post_notification_raises_bad_request_if_not_valid_notification_type(cli assert 'The requested URL was not found on the server.' in error_json['message'] -@pytest.mark.parametrize("reference", [None, "reference_from_client"]) -def test_post_sms_notification_with_invalid_reply_to_email_id( +@pytest.mark.parametrize("notification_type", + ['sms', 'email']) +def test_post_notification_with_wrong_type_of_sender( client, - sample_template_with_placeholders, - reference, + sample_template, + sample_email_template, + notification_type, fake_uuid): - data = { - 'phone_number': '+447700900855', - 'template_id': str(sample_template_with_placeholders.id), - 'personalisation': {' Name': 'Jo'}, - 'email_reply_to_id': fake_uuid - } - if reference: - data.update({"reference": reference}) - auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + if notification_type == EMAIL_TYPE: + template = sample_email_template + form_label = 'sms_sender_id' + data = { + 'email_address': 'test@test.com', + 'template_id': str(sample_email_template.id), + form_label: fake_uuid + } + elif notification_type == SMS_TYPE: + template = sample_template + form_label = 'email_reply_to_id' + data = { + 'phone_number': '+447700900855', + 'template_id': str(template.id), + form_label: fake_uuid + } + auth_header = create_authorization_header(service_id=template.service_id) response = client.post( - path='/v2/notifications/sms', + path='/v2/notifications/{}'.format(notification_type), 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 'email_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 '{} 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'] @@ -592,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