darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-02-01 23:55:58 -05:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3020 from alphagov/revert-3000-add-save-sms-api-task

Browse Source 
Revert "Add a task to save-api-sms for high volume services."
This commit is contained in:
Rebecca Law
2020-10-27 16:28:16 +00:00
committed by GitHub
parent ef416a64ed 06ff1bf596
commit f47a1af305
7 changed files with 167 additions and 236 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
app/celery/tasks.py
View File

@@ -287,23 +287,13 @@ def save_email(self,
@notify_celery.task(bind=True, name="save-api-email", max_retries=5, default_retry_delay=300)
@statsd(namespace="tasks")
def save_api_email(self, encrypted_notification):
def save_api_email(self,
encrypted_notification,
):
save_api_email_or_sms(self, encrypted_notification)
@notify_celery.task(bind=True, name="save-api-sms", max_retries=5, default_retry_delay=300)
@statsd(namespace="tasks")
def save_api_sms(self, encrypted_notification):
save_api_email_or_sms(self, encrypted_notification)
def save_api_email_or_sms(self, encrypted_notification):
notification = encryption.decrypt(encrypted_notification)
service = dao_fetch_service_by_id(notification['service_id'])
q = QueueNames.SEND_EMAIL if notification['notification_type'] == EMAIL_TYPE else QueueNames.SEND_SMS
provider_task = provider_tasks.deliver_email if notification['notification_type'] == EMAIL_TYPE \
else provider_tasks.deliver_sms
try:
persist_notification(
@@ -313,7 +303,7 @@ def save_api_email_or_sms(self, encrypted_notification):
recipient=notification['to'],
service=service,
personalisation=notification.get('personalisation'),
notification_type=notification['notification_type'],
notification_type=EMAIL_TYPE,
client_reference=notification['client_reference'],
api_key_id=notification.get('api_key_id'),
key_type=KEY_TYPE_NORMAL,
@@ -323,16 +313,14 @@ def save_api_email_or_sms(self, encrypted_notification):
document_download_count=notification['document_download_count']
)
q = q if not service.research_mode else QueueNames.RESEARCH_MODE
provider_task.apply_async(
q = QueueNames.SEND_EMAIL if not service.research_mode else QueueNames.RESEARCH_MODE
provider_tasks.deliver_email.apply_async(
[notification['id']],
queue=q
)
current_app.logger.debug(
f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue."
)
current_app.logger.debug(f"Email {notification['id']} has been persisted and sent to delivery queue.")
except IntegrityError:
current_app.logger.info(f"{notification['notification_type']} {notification['id']} already exists.")
current_app.logger.info(f"Email {notification['id']} already exists.")
except SQLAlchemyError:

4
app/config.py
View File

@@ -32,7 +32,6 @@ class QueueNames(object):
ANTIVIRUS = 'antivirus-tasks'
SANITISE_LETTERS = 'sanitise-letter-tasks'
SAVE_API_EMAIL = 'save-api-email-tasks'
SAVE_API_SMS = 'save-api-sms-tasks'
@staticmethod
def all_queues():
@@ -51,8 +50,7 @@ class QueueNames(object):
QueueNames.CALLBACKS,
QueueNames.LETTERS,
QueueNames.SMS_CALLBACKS,
QueueNames.SAVE_API_EMAIL,
QueueNames.SAVE_API_SMS
QueueNames.SAVE_API_EMAIL
]

25
app/v2/notifications/post_notifications.py
View File

@@ -18,7 +18,7 @@ from app import (
)
from app.celery.letters_pdf_tasks import get_pdf_for_templated_letter, sanitise_letter
from app.celery.research_mode_tasks import create_fake_letter_response_file
from app.celery.tasks import save_api_email, save_api_sms
from app.celery.tasks import save_api_email
from app.clients.document_download import DocumentDownloadError
from app.config import QueueNames, TaskNames
from app.dao.templates_dao import get_precompiled_letter_template
@@ -69,6 +69,7 @@ from app.v2.notifications.notification_schemas import (
)
from app.v2.utils import get_valid_json
POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS = Histogram(
'post_notification_json_parse_duration_seconds',
'Time taken to parse and validate post request json',
@@ -200,16 +201,14 @@ def process_sms_or_email_notification(
template_with_content=template_with_content
)
if service.id in current_app.config.get('HIGH_VOLUME_SERVICE') \
and api_user.key_type == KEY_TYPE_NORMAL \
and notification_type in [EMAIL_TYPE, SMS_TYPE]:
# Put service with high volumes of notifications onto a queue
if service.id in current_app.config.get('HIGH_VOLUME_SERVICE') and api_user.key_type == KEY_TYPE_NORMAL \
and notification_type == EMAIL_TYPE:
# Put GOV.UK Email notifications onto a queue
# To take the pressure off the db for API requests put the notification for our high volume service onto a queue
# the task will then save the notification, then call send_notification_to_queue.
# NOTE: The high volume service should be aware that the notification is not immediately
# available by a GET request, it is recommend they use callbacks to keep track of status updates.
# We know that this team does not use the GET request, but relies on callbacks to get the status updates.
try:
save_email_or_sms_to_queue(
save_email_to_queue(
form=form,
notification_id=str(notification_id),
notification_type=notification_type,
@@ -259,7 +258,7 @@ def process_sms_or_email_notification(
return resp
def save_email_or_sms_to_queue(
def save_email_to_queue(
*,
notification_id,
form,
@@ -275,7 +274,7 @@ def save_email_or_sms_to_queue(
"id": notification_id,
"template_id": str(template.id),
"template_version": template.version,
"to": form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number'],
"to": form['email_address'],
"service_id": str(service_id),
"personalisation": personalisation,
"notification_type": notification_type,
@@ -291,11 +290,7 @@ def save_email_or_sms_to_queue(
data
)
if notification_type == EMAIL_TYPE:
save_api_email.apply_async([encrypted], queue=QueueNames.SAVE_API_EMAIL)
elif notification_type == SMS_TYPE:
save_api_sms.apply_async([encrypted], queue=QueueNames.SAVE_API_SMS)
save_api_email.apply_async([encrypted], queue=QueueNames.SAVE_API_EMAIL)
return Notification(**data)

2
scripts/paas_app_wrapper.sh
View File

@@ -51,7 +51,7 @@ case $NOTIFY_APP_NAME in
;;
delivery-worker-save-api-notifications)
exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \
-Q save-api-email-tasks, save-api-sms-tasks 2> /dev/null
-Q save-api-email-tasks 2> /dev/null
;;
delivery-celery-beat)
exec scripts/run_app_paas.sh celery -A run_celery.notify_celery beat --loglevel=INFO

120
tests/app/celery/test_tasks.py
View File

@@ -33,9 +33,8 @@ from app.celery.tasks import (
s3,
send_inbound_sms_to_service,
process_returned_letters_list,
get_recipient_csv_and_template_and_sender_id,
save_api_email,
save_api_sms
get_recipient_csv_and_template_and_sender_id,
)
from app.config import QueueNames
from app.dao import jobs_dao, service_email_reply_to_dao, service_sms_sender_dao
@@ -1792,66 +1791,17 @@ def test_process_returned_letters_populates_returned_letters_table(
@freeze_time('2020-03-25 14:30')
@pytest.mark.parametrize('notification_type', ['sms', 'email'])
def test_save_api_email_or_sms(mocker, sample_service, notification_type):
template = create_template(sample_service) if notification_type == SMS_TYPE \
else create_template(sample_service, template_type=EMAIL_TYPE)
mock_provider_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async')
api_key = create_api_key(service=template.service)
def test_save_api_email(sample_email_template, mocker):
mock_send_email_to_provider = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
api_key = create_api_key(service=sample_email_template.service)
data = {
"id": str(uuid.uuid4()),
"template_id": str(template.id),
"template_version": template.version,
"service_id": str(template.service_id),
"template_id": str(sample_email_template.id),
"template_version": sample_email_template.version,
"to": "jane.citizen@example.com",
"service_id": str(sample_email_template.service_id),
"personalisation": None,
"notification_type": template.template_type,
"api_key_id": str(api_key.id),
"key_type": api_key.key_type,
"client_reference": 'our email',
"reply_to_text": None,
"document_download_count": 0,
"status": NOTIFICATION_CREATED,
"created_at": datetime.utcnow().strftime(DATETIME_FORMAT),
}
if notification_type == EMAIL_TYPE:
data.update({"to": "jane.citizen@example.com"})
expected_queue = QueueNames.SEND_EMAIL
else:
data.update({"to": "+447700900855"})
expected_queue = QueueNames.SEND_SMS
encrypted = encryption.encrypt(
data
)
assert len(Notification.query.all()) == 0
if notification_type == EMAIL_TYPE:
save_api_email(encrypted_notification=encrypted)
else:
save_api_sms(encrypted_notification=encrypted)
notifications = Notification.query.all()
assert len(notifications) == 1
assert str(notifications[0].id) == data['id']
assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
assert notifications[0].notification_type == notification_type
mock_provider_task.assert_called_once_with([data['id']], queue=expected_queue)
@freeze_time('2020-03-25 14:30')
@pytest.mark.parametrize('notification_type', ['sms', 'email'])
def test_save_api_email_dont_retry_if_notification_already_exists(sample_service, mocker, notification_type):
template = create_template(sample_service) if notification_type == SMS_TYPE \
else create_template(sample_service, template_type=EMAIL_TYPE)
mock_provider_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async')
api_key = create_api_key(service=template.service)
data = {
"id": str(uuid.uuid4()),
"template_id": str(template.id),
"template_version": template.version,
"service_id": str(template.service_id),
"personalisation": None,
"notification_type": template.template_type,
"notification_type": sample_email_template.template_type,
"api_key_id": str(api_key.id),
"key_type": api_key.key_type,
"client_reference": 'our email',
@@ -1861,32 +1811,52 @@ def test_save_api_email_dont_retry_if_notification_already_exists(sample_service
"created_at": datetime.utcnow().strftime(DATETIME_FORMAT),
}
if notification_type == EMAIL_TYPE:
data.update({"to": "jane.citizen@example.com"})
expected_queue = QueueNames.SEND_EMAIL
else:
data.update({"to": "+447700900855"})
expected_queue = QueueNames.SEND_SMS
encrypted = encryption.encrypt(
data
)
assert len(Notification.query.all()) == 0
save_api_email(encrypted)
notifications = Notification.query.all()
assert len(notifications) == 1
assert str(notifications[0].id) == data['id']
assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
mock_send_email_to_provider.assert_called_once_with([data['id']], queue=QueueNames.SEND_EMAIL)
@freeze_time('2020-03-25 14:30')
def test_save_api_email_dont_retry_if_notification_already_exists(sample_email_template, mocker):
mock_send_email_to_provider = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
api_key = create_api_key(service=sample_email_template.service)
data = {
"id": str(uuid.uuid4()),
"template_id": str(sample_email_template.id),
"template_version": sample_email_template.version,
"to": "jane.citizen@example.com",
"service_id": str(sample_email_template.service_id),
"personalisation": None,
"notification_type": sample_email_template.template_type,
"api_key_id": str(api_key.id),
"key_type": api_key.key_type,
"client_reference": 'our email',
"reply_to_text": "our.email@gov.uk",
"document_download_count": 0,
"status": NOTIFICATION_CREATED,
"created_at": datetime.utcnow().strftime(DATETIME_FORMAT),
}
encrypted = encryption.encrypt(
data
)
assert len(Notification.query.all()) == 0
if notification_type == EMAIL_TYPE:
save_api_email(encrypted_notification=encrypted)
else:
save_api_sms(encrypted_notification=encrypted)
save_api_email(encrypted)
notifications = Notification.query.all()
assert len(notifications) == 1
# call the task again with the same notification
if notification_type == EMAIL_TYPE:
save_api_email(encrypted_notification=encrypted)
else:
save_api_sms(encrypted_notification=encrypted)
save_api_email(encrypted)
notifications = Notification.query.all()
assert len(notifications) == 1
assert str(notifications[0].id) == data['id']
assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
# should only have sent the notification once.
mock_provider_task.assert_called_once_with([data['id']], queue=expected_queue)
mock_send_email_to_provider.assert_called_once_with([data['id']], queue=QueueNames.SEND_EMAIL)

5
tests/app/test_config.py
View File

@@ -60,7 +60,7 @@ def test_load_config_if_cloudfoundry_not_available(reload_config):
def test_queue_names_all_queues_correct():
# Need to ensure that all_queues() only returns queue names used in API
queues = QueueNames.all_queues()
assert len(queues) == 16
assert len(queues) == 15
assert set([
QueueNames.PRIORITY,
QueueNames.PERIODIC,
@@ -76,6 +76,5 @@ def test_queue_names_all_queues_correct():
QueueNames.CALLBACKS,
QueueNames.LETTERS,
QueueNames.SMS_CALLBACKS,
QueueNames.SAVE_API_EMAIL,
QueueNames.SAVE_API_SMS
QueueNames.SAVE_API_EMAIL
]) == set(queues)

217
tests/app/v2/notifications/test_post_notifications.py
View File

@@ -29,7 +29,6 @@ from tests.app.db import (
create_service_with_inbound_number,
create_api_key
)
from tests.conftest import set_config_values
@pytest.mark.parametrize("reference", [None, "reference_from_client"])
@@ -1030,147 +1029,129 @@ def test_post_email_notification_when_data_is_empty_returns_400(client, sample_s
assert error_msg == 'email_address is a required property'
@pytest.mark.parametrize("notification_type", ("email", "sms"))
def test_post_notifications_saves_email_or_sms_to_queue(client, notify_db_session, mocker, notification_type):
save_task = mocker.patch(f"app.celery.tasks.save_api_{notification_type}.apply_async")
mock_send_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async')
def test_post_notifications_saves_email_to_queue(client, notify_db_session, mocker):
save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][0],
service_name='high volume service',
)
with set_config_values(current_app, {
'HIGH_VOLUME_SERVICE': [str(service.id)],
template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE)
data = {
"email_address": "joe.citizen@example.com",
"template_id": template.id,
"personalisation": {"message": "Dear citizen, have a nice day"}
}
response = client.post(
path='/v2/notifications/email',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
)
}):
template = create_template(service=service, content='((message))', template_type=notification_type)
data = {
"template_id": template.id,
"personalisation": {"message": "Dear citizen, have a nice day"}
}
data.update({"email_address": "joe.citizen@example.com"}) if notification_type == EMAIL_TYPE \
else data.update({"phone_number": "+447700900855"})
json_resp = response.get_json()
response = client.post(
path=f'/v2/notifications/{notification_type}',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
)
json_resp = response.get_json()
assert response.status_code == 201
assert json_resp['id']
assert json_resp['content']['body'] == "Dear citizen, have a nice day"
assert json_resp['template']['id'] == str(template.id)
save_task.assert_called_once_with([mock.ANY], queue=f'save-api-{notification_type}-tasks')
assert not mock_send_task.called
assert len(Notification.query.all()) == 0
assert response.status_code == 201
assert json_resp['id']
assert json_resp['content']['body'] == "Dear citizen, have a nice day"
assert json_resp['template']['id'] == str(template.id)
save_email_task.assert_called_once_with([mock.ANY], queue='save-api-email-tasks')
assert not mock_send_task.called
assert len(Notification.query.all()) == 0
@pytest.mark.parametrize("notification_type", ("email", "sms"))
def test_post_notifications_saves_email_or_sms_normally_if_saving_to_queue_fails(
client, notify_db_session, mocker, notification_type
):
save_task = mocker.patch(
f"app.celery.tasks.save_api_{notification_type}.apply_async",
def test_post_notifications_saves_email_normally_if_save_email_to_queue_fails(client, notify_db_session, mocker):
save_email_task = mocker.patch(
"app.celery.tasks.save_api_email.apply_async",
side_effect=SQSError({'some': 'json'}, 'some opname')
)
mock_send_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async')
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][1],
service_name='high volume service',
)
with set_config_values(current_app, {
'HIGH_VOLUME_SERVICE': [str(service.id)],
template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE)
data = {
"email_address": "joe.citizen@example.com",
"template_id": template.id,
"personalisation": {"message": "Dear citizen, have a nice day"}
}
response = client.post(
path='/v2/notifications/email',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
)
}):
template = create_template(service=service, content='((message))', template_type=notification_type)
data = {
"template_id": template.id,
"personalisation": {"message": "Dear citizen, have a nice day"}
}
data.update({"email_address": "joe.citizen@example.com"}) if notification_type == EMAIL_TYPE \
else data.update({"phone_number": "+447700900855"})
json_resp = response.get_json()
response = client.post(
path=f'/v2/notifications/{notification_type}',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
)
json_resp = response.get_json()
assert response.status_code == 201
assert json_resp['id']
assert json_resp['content']['body'] == "Dear citizen, have a nice day"
assert json_resp['template']['id'] == str(template.id)
save_task.assert_called_once_with([mock.ANY], queue=f'save-api-{notification_type}-tasks')
mock_send_task.assert_called_once_with([json_resp['id']], queue=f'send-{notification_type}-tasks')
assert Notification.query.count() == 1
assert response.status_code == 201
assert json_resp['id']
assert json_resp['content']['body'] == "Dear citizen, have a nice day"
assert json_resp['template']['id'] == str(template.id)
# save email
save_email_task.assert_called_once_with([mock.ANY], queue='save-api-email-tasks')
mock_send_task.assert_called_once_with([json_resp['id']], queue='send-email-tasks')
assert Notification.query.count() == 1
@pytest.mark.parametrize("notification_type", ("email", "sms"))
def test_post_notifications_doesnt_use_save_queue_for_test_notifications(
client, notify_db_session, mocker, notification_type
):
save_task = mocker.patch(f"app.celery.tasks.save_api_{notification_type}.apply_async")
mock_send_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async')
def test_post_notifications_doesnt_save_email_to_queue_for_test_emails(client, notify_db_session, mocker):
save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][2],
service_name='high volume service',
)
with set_config_values(current_app, {
'HIGH_VOLUME_SERVICE': [str(service.id)],
template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE)
data = {
"email_address": "joe.citizen@example.com",
"template_id": template.id,
"personalisation": {"message": "Dear citizen, have a nice day"}
}
response = client.post(
path='/v2/notifications/email',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'),
create_authorization_header(service_id=service.id, key_type='test')]
)
}):
template = create_template(service=service, content='((message))', template_type=notification_type)
data = {
"template_id": template.id,
"personalisation": {"message": "Dear citizen, have a nice day"}
}
data.update({"email_address": "joe.citizen@example.com"}) if notification_type == EMAIL_TYPE \
else data.update({"phone_number": "+447700900855"})
response = client.post(
path=f'/v2/notifications/{notification_type}',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'),
create_authorization_header(service_id=service.id, key_type='test')]
)
json_resp = response.get_json()
json_resp = response.get_json()
assert response.status_code == 201
assert json_resp['id']
assert json_resp['content']['body'] == "Dear citizen, have a nice day"
assert json_resp['template']['id'] == str(template.id)
assert mock_send_task.called
assert not save_task.called
assert len(Notification.query.all()) == 1
assert response.status_code == 201
assert json_resp['id']
assert json_resp['content']['body'] == "Dear citizen, have a nice day"
assert json_resp['template']['id'] == str(template.id)
assert mock_send_task.called
assert not save_email_task.called
assert len(Notification.query.all()) == 1
def test_post_notification_does_not_use_save_queue_for_letters(client, sample_letter_template, mocker):
mock_save = mocker.patch("app.v2.notifications.post_notifications.save_email_or_sms_to_queue")
mock_create_pdf_task = mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async')
def test_post_notifications_doesnt_save_email_to_queue_for_sms(client, notify_db_session, mocker):
save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
with set_config_values(current_app, {
'HIGH_VOLUME_SERVICE': [str(sample_letter_template.service_id)],
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][3],
service_name='high volume service',
)
template = create_template(service=service, content='((message))', template_type=SMS_TYPE)
data = {
"phone_number": '+447700900855',
"template_id": template.id,
"personalisation": {"message": "Dear citizen, have a nice day"}
}
response = client.post(
path='/v2/notifications/sms',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
)
}):
data = {
'template_id': str(sample_letter_template.id),
'personalisation': {
'address_line_1': 'Her Royal Highness Queen Elizabeth II',
'address_line_2': 'Buckingham Palace',
'address_line_3': 'London',
'postcode': 'SW1 1AA',
}
}
response = client.post(
path='/v2/notifications/letter',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'),
create_authorization_header(service_id=sample_letter_template.service_id)]
)
assert response.status_code == 201
json_resp = response.get_json()
assert not mock_save.called
mock_create_pdf_task.assert_called_once_with([str(json_resp['id'])], queue='create-letters-pdf-tasks')
json_resp = response.get_json()
assert response.status_code == 201
assert json_resp['id']
assert mock_send_task.called
assert not save_email_task.called
assert len(Notification.query.all()) == 1