diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 71a8c8d02..1900fc250 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -3,13 +3,10 @@ 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 +from app.dao.email_branding_dao import dao_get_email_branding_by_id from app.dao.notifications_dao import ( dao_update_notification ) @@ -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,10 +27,11 @@ 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) if not service.active: technical_failure(notification=notification) @@ -43,7 +40,9 @@ 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_model = SerialisedTemplate.from_id_and_service_id( + template_id=notification.template_id, service_id=service.id, version=notification.template_version + ) template = SMSMessageTemplate( template_model.__dict__, @@ -51,7 +50,8 @@ def send_sms_to_provider(notification): prefix=service.name, show_prefix=service.prefix_sms, ) - + created_at = notification.created_at + key_type = notification.key_type 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) @@ -59,7 +59,7 @@ def send_sms_to_provider(notification): 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,10 +73,10 @@ 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: + if 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) @@ -87,14 +87,17 @@ def send_sms_to_provider(notification): def send_email_to_provider(notification): - service = notification.service + service = SerialisedService.from_id(notification.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, @@ -106,7 +109,8 @@ def send_email_to_provider(notification): template_dict, values=notification.personalisation ) - + created_at = notification.created_at + key_type = notification.key_type if service.research_mode or notification.key_type == KEY_TYPE_TEST: notification.reference = str(create_uuid()) update_notification_to_sending(notification, provider) @@ -115,22 +119,19 @@ def send_email_to_provider(notification): 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() - created_at).total_seconds() - delta_seconds = (datetime.utcnow() - notification.created_at).total_seconds() - - if notification.key_type == KEY_TYPE_TEST: + if 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) @@ -190,25 +191,28 @@ def get_logo_url(base_url, logo_file): def get_html_email_options(service): - if service.email_branding is None: return { 'govuk_banner': True, 'brand_banner': False, } + if isinstance(service, SerialisedService): + branding = dao_get_email_branding_by_id(service.email_branding) + else: + branding = service.email_branding logo_url = get_logo_url( current_app.config['ADMIN_BASE_URL'], - service.email_branding.logo - ) if service.email_branding.logo else None + branding.logo + ) if branding.logo else None return { - 'govuk_banner': service.email_branding.brand_type == BRANDING_BOTH, - 'brand_banner': service.email_branding.brand_type == BRANDING_ORG_BANNER, - 'brand_colour': service.email_branding.colour, + 'govuk_banner': branding.brand_type == BRANDING_BOTH, + 'brand_banner': branding.brand_type == BRANDING_ORG_BANNER, + 'brand_colour': branding.colour, 'brand_logo': logo_url, - 'brand_text': service.email_branding.text, - 'brand_name': service.email_branding.name, + 'brand_text': branding.text, + 'brand_name': branding.name, } 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..1cad72af3 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 @@ -13,6 +14,7 @@ from app import notification_provider_clients, mmg_client, firetext_client from app.dao import notifications_dao from app.dao.provider_details_dao import get_provider_details_by_identifier from app.delivery import send_to_providers +from app.delivery.send_to_providers import get_html_email_options, get_logo_url from app.exceptions import NotificationTechnicalFailureException from app.models import ( Notification, @@ -24,13 +26,14 @@ from app.models import ( BRANDING_BOTH, BRANDING_ORG_BANNER ) +from app.serialised_models import SerialisedService from tests.app.db import ( create_service, create_template, create_notification, create_reply_to_email, create_service_sms_sender, - create_service_with_defined_sms_sender + create_service_with_defined_sms_sender, create_email_branding ) @@ -105,7 +108,9 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( db_notification = create_notification(template=sample_sms_template_with_html, to_field="+447234123123", personalisation={"name": "Jo"}, status='created', - reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender()) + reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), + normalised_to="447234123123" + ) mocker.patch('app.mmg_client.send_sms') @@ -114,7 +119,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="447234123123", content="Sample service: Hello Jo\nHere is some HTML & entities", reference=str(db_notification.id), sender=current_app.config['FROM_NUMBER'] @@ -136,7 +141,8 @@ 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", - personalisation={'name': 'Jo'} + personalisation={'name': 'Jo'}, + normalised_to="jo.smith@example.com", ) mocker.patch('app.aws_ses_client.send_email', return_value='reference') @@ -194,7 +200,8 @@ 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', - reply_to_text=sample_template.service.get_default_sms_sender()) + reply_to_text=sample_template.service.get_default_sms_sender(), + normalised_to='447234123123') mocker.patch('app.mmg_client.send_sms') @@ -634,7 +641,8 @@ def test_should_send_sms_to_international_providers( personalisation={"name": "Jo"}, status='created', international=False, - reply_to_text=sample_template.service.get_default_sms_sender() + reply_to_text=sample_template.service.get_default_sms_sender(), + normalised_to="447234123999" ) notification_international = create_notification( @@ -643,7 +651,8 @@ def test_should_send_sms_to_international_providers( personalisation={"name": "Jo"}, status='created', international=True, - reply_to_text=sample_template.service.get_default_sms_sender() + reply_to_text=sample_template.service.get_default_sms_sender(), + normalised_to='601117224412' ) send_to_providers.send_sms_to_provider( notification_uk @@ -726,49 +735,133 @@ 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) + + +def test_get_html_email_options_return_email_branding_from_serialised_service( + sample_service +): + branding = create_email_branding() + sample_service.email_branding = branding + service = SerialisedService.from_id(sample_service.id) + email_options = get_html_email_options(service) + assert email_options is not None + assert email_options == {'govuk_banner': branding.brand_type == BRANDING_BOTH, + 'brand_banner': branding.brand_type == BRANDING_ORG_BANNER, + 'brand_colour': branding.colour, + 'brand_logo': get_logo_url(current_app.config['ADMIN_BASE_URL'], branding.logo), + 'brand_text': branding.text, + 'brand_name': branding.name, + } + + +def test_get_html_email_options_add_email_branding_from_service(sample_service): + branding = create_email_branding() + sample_service.email_branding = branding + email_options = get_html_email_options(sample_service) + assert email_options is not None + assert email_options == {'govuk_banner': branding.brand_type == BRANDING_BOTH, + 'brand_banner': branding.brand_type == BRANDING_ORG_BANNER, + 'brand_colour': branding.colour, + 'brand_logo': get_logo_url(current_app.config['ADMIN_BASE_URL'], branding.logo), + 'brand_text': branding.text, + 'brand_name': branding.name, + }