diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 71a8c8d02..b30e1aeb2 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 @@ -35,7 +32,7 @@ from app.models import ( def send_sms_to_provider(notification): service = notification.service - + service_id = service.id if not service.active: technical_failure(notification=notification) return @@ -52,14 +49,14 @@ def send_sms_to_provider(notification): 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,14 +70,14 @@ 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) @@ -88,6 +85,7 @@ def send_sms_to_provider(notification): def send_email_to_provider(notification): service = notification.service + service_id = service.id if not service.active: technical_failure(notification=notification) return @@ -107,6 +105,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 +113,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/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 4ddfa0563..8be87fd6a 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -103,7 +103,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 +116,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 +138,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 +197,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 +216,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 +635,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 +645,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, @@ -724,51 +730,3 @@ def test_send_email_to_provider_uses_reply_to_from_notification( html_body=ANY, reply_to_address="test@test.com" ) - - -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') - - send_to_providers.send_sms_to_provider(sample_notification) - - assert send_mock.call_args[1]['to'] == '447123123123' - - -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') - - send_to_providers.send_email_to_provider(sample_email_notification) - - # to_addresses - send_mock.assert_called_once_with( - ANY, - # to_addresses - 'test@example.com', - ANY, - body=ANY, - html_body=ANY, - reply_to_address=ANY, - )