From a32e418bab318646464806aa0b1514143d8801d9 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 27 Oct 2020 11:10:35 +0000 Subject: [PATCH] Make sure that letters can not use the high volume service code path. This code is already restricted to SMS and email, so this is only to make it obvious if there is a code refactor down the line. Perhaps this is overkill and we back out this commit. --- app/v2/notifications/post_notifications.py | 12 +- .../notifications/test_post_notifications.py | 227 ++++++++++-------- 2 files changed, 129 insertions(+), 110 deletions(-) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 7d6414c7e..97f6dfeac 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -69,7 +69,6 @@ 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', @@ -201,11 +200,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: - # Put GOV.UK Email 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 in [EMAIL_TYPE, SMS_TYPE]: + # Put service with high volumes of 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. - # We know that this team does not use the GET request, but relies on callbacks to get the status updates. + # 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. try: save_email_or_sms_to_queue( form=form, @@ -291,7 +293,7 @@ def save_email_or_sms_to_queue( if notification_type == EMAIL_TYPE: save_api_email.apply_async([encrypted], queue=QueueNames.SAVE_API_EMAIL) - else: + elif notification_type == SMS_TYPE: save_api_sms.apply_async([encrypted], queue=QueueNames.SAVE_API_SMS) return Notification(**data) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 148ad2ca0..58da17c74 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -29,6 +29,7 @@ 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"]) @@ -1029,131 +1030,147 @@ def test_post_email_notification_when_data_is_empty_returns_400(client, sample_s assert error_msg == 'email_address is a required property' -def test_post_notifications_saves_sms_to_queue(client, notify_db_session, mocker): - save_task = mocker.patch("app.celery.tasks.save_api_sms.apply_async") - mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') +@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') service = create_service( - service_id=current_app.config['HIGH_VOLUME_SERVICE'][0], - service_name=f'high volume service', - ) - template = create_template(service=service, content='((message))') - data = { - "phone_number": "+447700900855", - "template_id": template.id, - "personalisation": {"message": "Dear citizen, have a nice day"} - } - - response = client.post( - path=f'/v2/notifications/sms', - 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='save-api-sms-tasks') - assert not mock_send_task.called - assert len(Notification.query.all()) == 0 - - -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'][1], service_name='high volume service', ) - 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)] - ) + with set_config_values(current_app, { + 'HIGH_VOLUME_SERVICE': [str(service.id)], - json_resp = response.get_json() + }): + 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"}) - 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 + 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 -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", +@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", side_effect=SQSError({'some': 'json'}, 'some opname') ) - mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + mock_send_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async') service = create_service( - service_id=current_app.config['HIGH_VOLUME_SERVICE'][2], service_name='high volume service', ) - 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)] - ) + with set_config_values(current_app, { + 'HIGH_VOLUME_SERVICE': [str(service.id)], - json_resp = response.get_json() + }): + 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"}) - 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 + 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 -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') - +@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') 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=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')] - ) + with set_config_values(current_app, { + 'HIGH_VOLUME_SERVICE': [str(service.id)], - json_resp = response.get_json() + }): + 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')] + ) - 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 + 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 + + +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') + + with set_config_values(current_app, { + 'HIGH_VOLUME_SERVICE': [str(sample_letter_template.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')