diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index e72772972..71a8c8d02 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -3,7 +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 @@ -15,6 +18,7 @@ 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, @@ -27,12 +31,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 = SerialisedService.from_id(notification.service_id) - service_id = service.id + service = notification.service + if not service.active: technical_failure(notification=notification) return @@ -40,25 +43,23 @@ def send_sms_to_provider(notification): if notification.status == 'created': provider = provider_to_use(SMS_TYPE, notification.international) - template_dict = SerialisedTemplate.from_id_and_service_id(template_id=notification.template_id, - service_id=service_id, - version=notification.template_version).__dict__ + template_model = dao_get_template_by_id(notification.template_id, notification.template_version) template = SMSMessageTemplate( - template_dict, + template_model.__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=notification.normalised_to, + to=validate_and_format_phone_number(notification.to, international=notification.international), content=str(template), reference=str(notification.id), sender=notification.reply_to_text @@ -72,31 +73,28 @@ def send_sms_to_provider(notification): notification.billable_units = template.fragment_count update_notification_to_sending(notification, provider) - delta_seconds = (datetime.utcnow() - created_at).total_seconds() + delta_seconds = (datetime.utcnow() - notification.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 = SerialisedService.from_id(notification.service_id) - service_id = service.id + service = notification.service if not service.active: technical_failure(notification=notification) return if notification.status == 'created': provider = provider_to_use(EMAIL_TYPE) - template_dict = SerialisedTemplate.from_id_and_service_id(template_id=notification.template_id, - service_id=service_id, - version=notification.template_version).__dict__ + template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__ html_email = HTMLEmailTemplate( template_dict, @@ -109,7 +107,6 @@ 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) @@ -117,24 +114,27 @@ 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, - notification.normalised_to, + validate_and_format_email_address(notification.to), plain_text_email.subject, body=str(plain_text_email), html_body=str(html_email), - reply_to_address=notification.reply_to_text, + reply_to_address=validate_and_format_email_address(email_reply_to) if email_reply_to else None, ) 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: 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 8f9c93d69..31870a3d5 100644 --- a/app/serialised_models.py +++ b/app/serialised_models.py @@ -75,7 +75,6 @@ class SerialisedTemplate(SerialisedModel): class SerialisedService(SerialisedModel): ALLOWED_PROPERTIES = { 'id', - 'name', 'active', 'contact_link', 'email_from', @@ -84,8 +83,6 @@ 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 ec27798b1..4ddfa0563 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -1,4 +1,3 @@ -import json import uuid from collections import namedtuple from datetime import datetime, timedelta @@ -104,9 +103,7 @@ 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", - normalised_to=validate_and_format_phone_number("+447234123123", True), - personalisation={"name": "Jo"}, + to_field="+447234123123", personalisation={"name": "Jo"}, status='created', reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender()) @@ -117,7 +114,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( ) mmg_client.send_sms.assert_called_once_with( - to=db_notification.normalised_to, + to=validate_and_format_phone_number("+447234123123"), content="Sample service: Hello Jo\nHere is some HTML & entities", reference=str(db_notification.id), sender=current_app.config['FROM_NUMBER'] @@ -139,7 +136,6 @@ 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'} ) @@ -198,7 +194,6 @@ 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') @@ -217,7 +212,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( ) mmg_client.send_sms.assert_called_once_with( - to=db_notification.normalised_to, + to=validate_and_format_phone_number("+447234123123"), content="Sample service: This is a template:\nwith a newline", reference=str(db_notification.id), sender=current_app.config['FROM_NUMBER'] @@ -636,7 +631,6 @@ 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, @@ -646,7 +640,6 @@ 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, @@ -733,102 +726,49 @@ def test_send_email_to_provider_uses_reply_to_from_notification( ) -def test_send_sms_to_provider_should_use_normalised_to( - mocker, client, sample_template -): +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' 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) - send_mock.assert_called_once_with(to=notification.normalised_to, - content=ANY, - reference=str(notification.id), - sender=notification.reply_to_text) + + send_to_providers.send_sms_to_provider(sample_notification) + + assert send_mock.call_args[1]['to'] == '447123123123' -def test_send_email_to_provider_should_user_normalised_to( - mocker, client, sample_email_template -): +def test_send_email_to_provider_should_format_email_address(sample_email_notification, mocker): + sample_email_notification.to = 'test@example.com\t' 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) - send_mock.assert_called_once_with(ANY, - notification.normalised_to, - ANY, - body=ANY, - html_body=ANY, - reply_to_address=notification.reply_to_text) + send_to_providers.send_email_to_provider(sample_email_notification) - -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'), - ], + # to_addresses + send_mock.assert_called_once_with( + ANY, + # to_addresses + 'test@example.com', + ANY, + body=ANY, + html_body=ANY, + reply_to_address=ANY, ) - 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)