Remove validate_and_format for email and phone numbers, using normalised_to instead because that function has already happened when persisting the notificaiton.

Remove 2 extra select queries after the update and commit. Once a transaction is committed SQLAlchemy will query for the db model if referenced after a commit.
This commit is contained in:
Rebecca Law
2021-02-11 13:51:15 +00:00
parent 8f99da525d
commit 2270832873
2 changed files with 21 additions and 67 deletions

View File

@@ -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)

View File

@@ -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 <em>some HTML</em> & 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,
)