diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 71a8c8d02..e72772972 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -3,10 +3,7 @@ from urllib import parse from datetime import datetime, timedelta from cachetools import TTLCache, cached from flask import current_app -from notifications_utils.recipients import ( - validate_and_format_phone_number, - validate_and_format_email_address -) + from notifications_utils.template import HTMLEmailTemplate, PlainTextEmailTemplate, SMSMessageTemplate from app import notification_provider_clients, statsd_client, create_uuid @@ -18,7 +15,6 @@ from app.dao.provider_details_dao import ( dao_reduce_sms_provider_priority ) from app.celery.research_mode_tasks import send_sms_response, send_email_response -from app.dao.templates_dao import dao_get_template_by_id from app.exceptions import NotificationTechnicalFailureException from app.models import ( SMS_TYPE, @@ -31,11 +27,12 @@ from app.models import ( NOTIFICATION_SENDING, NOTIFICATION_STATUS_TYPES_COMPLETED ) +from app.serialised_models import SerialisedTemplate, SerialisedService def send_sms_to_provider(notification): - service = notification.service - + service = SerialisedService.from_id(notification.service_id) + service_id = service.id if not service.active: technical_failure(notification=notification) return @@ -43,23 +40,25 @@ def send_sms_to_provider(notification): if notification.status == 'created': provider = provider_to_use(SMS_TYPE, notification.international) - template_model = dao_get_template_by_id(notification.template_id, notification.template_version) + template_dict = SerialisedTemplate.from_id_and_service_id(template_id=notification.template_id, + service_id=service_id, + version=notification.template_version).__dict__ template = SMSMessageTemplate( - template_model.__dict__, + template_dict, values=notification.personalisation, prefix=service.name, show_prefix=service.prefix_sms, ) + created_at = notification.created_at if service.research_mode or notification.key_type == KEY_TYPE_TEST: update_notification_to_sending(notification, provider) send_sms_response(provider.get_name(), str(notification.id), notification.to) - else: try: provider.send_sms( - to=validate_and_format_phone_number(notification.to, international=notification.international), + to=notification.normalised_to, content=str(template), reference=str(notification.id), sender=notification.reply_to_text @@ -73,28 +72,31 @@ def send_sms_to_provider(notification): notification.billable_units = template.fragment_count update_notification_to_sending(notification, provider) - delta_seconds = (datetime.utcnow() - notification.created_at).total_seconds() + delta_seconds = (datetime.utcnow() - created_at).total_seconds() statsd_client.timing("sms.total-time", delta_seconds) if notification.key_type == KEY_TYPE_TEST: statsd_client.timing("sms.test-key.total-time", delta_seconds) else: statsd_client.timing("sms.live-key.total-time", delta_seconds) - if str(service.id) in current_app.config.get('HIGH_VOLUME_SERVICE'): + if str(service_id) in current_app.config.get('HIGH_VOLUME_SERVICE'): statsd_client.timing("sms.live-key.high-volume.total-time", delta_seconds) else: statsd_client.timing("sms.live-key.not-high-volume.total-time", delta_seconds) def send_email_to_provider(notification): - service = notification.service + service = SerialisedService.from_id(notification.service_id) + service_id = service.id if not service.active: technical_failure(notification=notification) return if notification.status == 'created': provider = provider_to_use(EMAIL_TYPE) - template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__ + template_dict = SerialisedTemplate.from_id_and_service_id(template_id=notification.template_id, + service_id=service_id, + version=notification.template_version).__dict__ html_email = HTMLEmailTemplate( template_dict, @@ -107,6 +109,7 @@ def send_email_to_provider(notification): values=notification.personalisation ) + created_at = notification.created_at if service.research_mode or notification.key_type == KEY_TYPE_TEST: notification.reference = str(create_uuid()) update_notification_to_sending(notification, provider) @@ -114,27 +117,24 @@ def send_email_to_provider(notification): else: from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']) - - email_reply_to = notification.reply_to_text - reference = provider.send_email( from_address, - validate_and_format_email_address(notification.to), + notification.normalised_to, plain_text_email.subject, body=str(plain_text_email), html_body=str(html_email), - reply_to_address=validate_and_format_email_address(email_reply_to) if email_reply_to else None, + reply_to_address=notification.reply_to_text, ) notification.reference = reference update_notification_to_sending(notification, provider) - delta_seconds = (datetime.utcnow() - notification.created_at).total_seconds() + delta_seconds = (datetime.utcnow() - created_at).total_seconds() if notification.key_type == KEY_TYPE_TEST: statsd_client.timing("email.test-key.total-time", delta_seconds) else: statsd_client.timing("email.live-key.total-time", delta_seconds) - if str(service.id) in current_app.config.get('HIGH_VOLUME_SERVICE'): + if str(service_id) in current_app.config.get('HIGH_VOLUME_SERVICE'): statsd_client.timing("email.live-key.high-volume.total-time", delta_seconds) else: statsd_client.timing("email.live-key.not-high-volume.total-time", delta_seconds) diff --git a/app/serialised_models.py b/app/serialised_models.py index 31870a3d5..8f9c93d69 100644 --- a/app/serialised_models.py +++ b/app/serialised_models.py @@ -75,6 +75,7 @@ class SerialisedTemplate(SerialisedModel): class SerialisedService(SerialisedModel): ALLOWED_PROPERTIES = { 'id', + 'name', 'active', 'contact_link', 'email_from', @@ -83,6 +84,8 @@ class SerialisedService(SerialisedModel): 'rate_limit', 'research_mode', 'restricted', + 'prefix_sms', + 'email_branding' } @classmethod diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 4ddfa0563..ec27798b1 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -1,3 +1,4 @@ +import json import uuid from collections import namedtuple from datetime import datetime, timedelta @@ -103,7 +104,9 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( mocker ): db_notification = create_notification(template=sample_sms_template_with_html, - to_field="+447234123123", personalisation={"name": "Jo"}, + to_field="+447234123123", + normalised_to=validate_and_format_phone_number("+447234123123", True), + personalisation={"name": "Jo"}, status='created', reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender()) @@ -114,7 +117,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( ) mmg_client.send_sms.assert_called_once_with( - to=validate_and_format_phone_number("+447234123123"), + to=db_notification.normalised_to, content="Sample service: Hello Jo\nHere is some HTML & entities", reference=str(db_notification.id), sender=current_app.config['FROM_NUMBER'] @@ -136,6 +139,7 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist db_notification = create_notification( template=sample_email_template_with_html, to_field="jo.smith@example.com", + normalised_to="jo.smith@example.com", personalisation={'name': 'Jo'} ) @@ -194,6 +198,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( sample_template, mocker): db_notification = create_notification(template=sample_template, to_field='+447234123123', status='created', + normalised_to=validate_and_format_phone_number("+447234123123"), reply_to_text=sample_template.service.get_default_sms_sender()) mocker.patch('app.mmg_client.send_sms') @@ -212,7 +217,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( ) mmg_client.send_sms.assert_called_once_with( - to=validate_and_format_phone_number("+447234123123"), + to=db_notification.normalised_to, content="Sample service: This is a template:\nwith a newline", reference=str(db_notification.id), sender=current_app.config['FROM_NUMBER'] @@ -631,6 +636,7 @@ def test_should_send_sms_to_international_providers( notification_uk = create_notification( template=sample_template, to_field="+447234123999", + normalised_to=validate_and_format_phone_number("+447234123999", True), personalisation={"name": "Jo"}, status='created', international=False, @@ -640,6 +646,7 @@ def test_should_send_sms_to_international_providers( notification_international = create_notification( template=sample_template, to_field="+6011-17224412", + normalised_to=validate_and_format_phone_number("+6011-17224412", international=True), personalisation={"name": "Jo"}, status='created', international=True, @@ -726,49 +733,102 @@ def test_send_email_to_provider_uses_reply_to_from_notification( ) -def test_send_email_to_provider_should_format_reply_to_email_address( - sample_email_template, - mocker): - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - - db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com\t") - - send_to_providers.send_email_to_provider( - db_notification, - ) - - app.aws_ses_client.send_email.assert_called_once_with( - ANY, - ANY, - ANY, - body=ANY, - html_body=ANY, - reply_to_address="test@test.com" - ) - - -def test_send_sms_to_provider_should_format_phone_number(sample_notification, mocker): - sample_notification.to = '+44 (7123) 123-123' +def test_send_sms_to_provider_should_use_normalised_to( + mocker, client, sample_template +): send_mock = mocker.patch('app.mmg_client.send_sms') - - send_to_providers.send_sms_to_provider(sample_notification) - - assert send_mock.call_args[1]['to'] == '447123123123' + notification = create_notification(template=sample_template, + to_field='+447700900855', + normalised_to='447700900855') + send_to_providers.send_sms_to_provider(notification) + send_mock.assert_called_once_with(to=notification.normalised_to, + content=ANY, + reference=str(notification.id), + sender=notification.reply_to_text) -def test_send_email_to_provider_should_format_email_address(sample_email_notification, mocker): - sample_email_notification.to = 'test@example.com\t' +def test_send_email_to_provider_should_user_normalised_to( + mocker, client, sample_email_template +): send_mock = mocker.patch('app.aws_ses_client.send_email', return_value='reference') + notification = create_notification(template=sample_email_template, + to_field='TEST@example.com', + normalised_to='test@example.com') - send_to_providers.send_email_to_provider(sample_email_notification) + send_to_providers.send_email_to_provider(notification) + send_mock.assert_called_once_with(ANY, + notification.normalised_to, + ANY, + body=ANY, + html_body=ANY, + reply_to_address=notification.reply_to_text) - # to_addresses - send_mock.assert_called_once_with( - ANY, - # to_addresses - 'test@example.com', - ANY, - body=ANY, - html_body=ANY, - reply_to_address=ANY, + +def test_send_sms_to_provider_should_return_template_if_found_in_redis( + mocker, client, sample_template +): + from app.schemas import service_schema, template_schema + service_dict = service_schema.dump(sample_template.service).data + template_dict = template_schema.dump(sample_template).data + + mocker.patch( + 'app.redis_store.get', + side_effect=[ + json.dumps({'data': service_dict}).encode('utf-8'), + json.dumps({'data': template_dict}).encode('utf-8'), + ], ) + mock_get_template = mocker.patch( + 'app.dao.templates_dao.dao_get_template_by_id_and_service_id' + ) + mock_get_service = mocker.patch( + 'app.dao.services_dao.dao_fetch_service_by_id' + ) + + send_mock = mocker.patch('app.mmg_client.send_sms') + notification = create_notification(template=sample_template, + to_field='+447700900855', + normalised_to='447700900855') + send_to_providers.send_sms_to_provider(notification) + assert mock_get_template.called is False + assert mock_get_service.called is False + send_mock.assert_called_once_with(to=notification.normalised_to, + content=ANY, + reference=str(notification.id), + sender=notification.reply_to_text) + + +def test_send_email_to_provider_should_return_template_if_found_in_redis( + mocker, client, sample_email_template +): + from app.schemas import service_schema, template_schema + service_dict = service_schema.dump(sample_email_template.service).data + template_dict = template_schema.dump(sample_email_template).data + + mocker.patch( + 'app.redis_store.get', + side_effect=[ + json.dumps({'data': service_dict}).encode('utf-8'), + json.dumps({'data': template_dict}).encode('utf-8'), + ], + ) + mock_get_template = mocker.patch( + 'app.dao.templates_dao.dao_get_template_by_id_and_service_id' + ) + mock_get_service = mocker.patch( + 'app.dao.services_dao.dao_fetch_service_by_id' + ) + send_mock = mocker.patch('app.aws_ses_client.send_email', return_value='reference') + notification = create_notification(template=sample_email_template, + to_field='TEST@example.com', + normalised_to='test@example.com') + + send_to_providers.send_email_to_provider(notification) + assert mock_get_template.called is False + assert mock_get_service.called is False + send_mock.assert_called_once_with(ANY, + notification.normalised_to, + ANY, + body=ANY, + html_body=ANY, + reply_to_address=notification.reply_to_text)