mirror of
https://github.com/GSA/notifications-api.git
synced 2025-12-24 09:21:39 -05:00
Merge pull request #3141 from alphagov/revert-3134-improve-sender-task
Revert "Improve sender task"
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <em>some HTML</em> & 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)
|
||||
|
||||
Reference in New Issue
Block a user