From 3dee4ad31040c5d4c5a2a57b95824555dc0c99ed Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 19 Oct 2020 13:29:05 +0100 Subject: [PATCH 1/2] Add a task to save-api-sms for high volume services. When we initially added a new task to persist the notifications for a high volume service we wanted to implement it as quickly as possible, so ignored SMS. This will allow a high volume service to send SMS, the SMS will be sent to a queue to then persist and send the SMS, similar to emails. At this point I haven't added a new application to consume the new save-api-sms-tasks. But we can add a separate application or be happy with how the app scales for both email and sms. --- app/celery/tasks.py | 30 +++++--- app/config.py | 4 +- app/v2/notifications/post_notifications.py | 17 +++-- scripts/paas_app_wrapper.sh | 2 +- tests/app/celery/test_tasks.py | 76 +++++++++++++------ tests/app/test_config.py | 5 +- .../notifications/test_post_notifications.py | 68 +++++++++-------- 7 files changed, 126 insertions(+), 76 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ba7111b19..a97364fa1 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -287,13 +287,23 @@ 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( @@ -303,7 +313,7 @@ def save_api_email(self, recipient=notification['to'], service=service, personalisation=notification.get('personalisation'), - notification_type=EMAIL_TYPE, + notification_type=notification['notification_type'], client_reference=notification['client_reference'], api_key_id=notification.get('api_key_id'), key_type=KEY_TYPE_NORMAL, @@ -313,14 +323,16 @@ def save_api_email(self, document_download_count=notification['document_download_count'] ) - q = QueueNames.SEND_EMAIL if not service.research_mode else QueueNames.RESEARCH_MODE - provider_tasks.deliver_email.apply_async( + q = q if not service.research_mode else QueueNames.RESEARCH_MODE + provider_task.apply_async( [notification['id']], queue=q ) - current_app.logger.debug(f"Email {notification['id']} has been persisted and sent to delivery queue.") + current_app.logger.debug( + f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue." + ) except IntegrityError: - current_app.logger.info(f"Email {notification['id']} already exists.") + current_app.logger.info(f"{notification['notification_type']} {notification['id']} already exists.") except SQLAlchemyError: diff --git a/app/config.py b/app/config.py index fedcc811e..5922659ab 100644 --- a/app/config.py +++ b/app/config.py @@ -32,6 +32,7 @@ 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(): @@ -50,7 +51,8 @@ class QueueNames(object): QueueNames.CALLBACKS, QueueNames.LETTERS, QueueNames.SMS_CALLBACKS, - QueueNames.SAVE_API_EMAIL + QueueNames.SAVE_API_EMAIL, + QueueNames.SAVE_API_SMS ] diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 139702b45..7d6414c7e 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -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 +from app.celery.tasks import save_api_email, save_api_sms from app.clients.document_download import DocumentDownloadError from app.config import QueueNames, TaskNames from app.dao.templates_dao import get_precompiled_letter_template @@ -201,14 +201,13 @@ 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 == EMAIL_TYPE: + 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 # 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. try: - save_email_to_queue( + save_email_or_sms_to_queue( form=form, notification_id=str(notification_id), notification_type=notification_type, @@ -258,7 +257,7 @@ def process_sms_or_email_notification( return resp -def save_email_to_queue( +def save_email_or_sms_to_queue( *, notification_id, form, @@ -274,7 +273,7 @@ def save_email_to_queue( "id": notification_id, "template_id": str(template.id), "template_version": template.version, - "to": form['email_address'], + "to": form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number'], "service_id": str(service_id), "personalisation": personalisation, "notification_type": notification_type, @@ -290,7 +289,11 @@ def save_email_to_queue( data ) - save_api_email.apply_async([encrypted], queue=QueueNames.SAVE_API_EMAIL) + if notification_type == EMAIL_TYPE: + save_api_email.apply_async([encrypted], queue=QueueNames.SAVE_API_EMAIL) + else: + save_api_sms.apply_async([encrypted], queue=QueueNames.SAVE_API_SMS) + return Notification(**data) diff --git a/scripts/paas_app_wrapper.sh b/scripts/paas_app_wrapper.sh index 9e4c7f3e0..f55f9cc3e 100755 --- a/scripts/paas_app_wrapper.sh +++ b/scripts/paas_app_wrapper.sh @@ -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 2> /dev/null + -Q save-api-email-tasks, save-api-sms-tasks 2> /dev/null ;; delivery-celery-beat) exec scripts/run_app_paas.sh celery -A run_celery.notify_celery beat --loglevel=INFO diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index f903016ad..d3bc64f09 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -33,8 +33,9 @@ from app.celery.tasks import ( s3, send_inbound_sms_to_service, process_returned_letters_list, - save_api_email, get_recipient_csv_and_template_and_sender_id, + save_api_email, + save_api_sms ) from app.config import QueueNames from app.dao import jobs_dao, service_email_reply_to_dao, service_sms_sender_dao @@ -1791,51 +1792,66 @@ def test_process_returned_letters_populates_returned_letters_table( @freeze_time('2020-03-25 14:30') -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) +@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) 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), + "template_id": str(template.id), + "template_version": template.version, + "service_id": str(template.service_id), "personalisation": None, - "notification_type": sample_email_template.template_type, + "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": "our.email@gov.uk", + "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 - save_api_email(encrypted) + 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) - mock_send_email_to_provider.assert_called_once_with([data['id']], queue=QueueNames.SEND_EMAIL) + 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') -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) +@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(sample_email_template.id), - "template_version": sample_email_template.version, - "to": "jane.citizen@example.com", - "service_id": str(sample_email_template.service_id), + "template_id": str(template.id), + "template_version": template.version, + "service_id": str(template.service_id), "personalisation": None, - "notification_type": sample_email_template.template_type, + "notification_type": template.template_type, "api_key_id": str(api_key.id), "key_type": api_key.key_type, "client_reference": 'our email', @@ -1845,18 +1861,32 @@ def test_save_api_email_dont_retry_if_notification_already_exists(sample_email_t "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) + + 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 # call the task again with the same notification - save_api_email(encrypted) + 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) # should only have sent the notification once. - mock_send_email_to_provider.assert_called_once_with([data['id']], queue=QueueNames.SEND_EMAIL) + mock_provider_task.assert_called_once_with([data['id']], queue=expected_queue) diff --git a/tests/app/test_config.py b/tests/app/test_config.py index fae5919fa..febd6d936 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -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) == 15 + assert len(queues) == 16 assert set([ QueueNames.PRIORITY, QueueNames.PERIODIC, @@ -76,5 +76,6 @@ def test_queue_names_all_queues_correct(): QueueNames.CALLBACKS, QueueNames.LETTERS, QueueNames.SMS_CALLBACKS, - QueueNames.SAVE_API_EMAIL + QueueNames.SAVE_API_EMAIL, + QueueNames.SAVE_API_SMS ]) == set(queues) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 5e3738155..148ad2ca0 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1029,12 +1029,44 @@ 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') + + 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'][0], + 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) @@ -1068,7 +1100,7 @@ def test_post_notifications_saves_email_normally_if_save_email_to_queue_fails(cl 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_id=current_app.config['HIGH_VOLUME_SERVICE'][2], service_name='high volume service', ) template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE) @@ -1100,7 +1132,7 @@ def test_post_notifications_doesnt_save_email_to_queue_for_test_emails(client, n 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_id=current_app.config['HIGH_VOLUME_SERVICE'][3], service_name='high volume service', ) template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE) @@ -1125,33 +1157,3 @@ def test_post_notifications_doesnt_save_email_to_queue_for_test_emails(client, n assert mock_send_task.called assert not save_email_task.called assert len(Notification.query.all()) == 1 - - -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') - - 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)] - ) - - 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 From a32e418bab318646464806aa0b1514143d8801d9 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 27 Oct 2020 11:10:35 +0000 Subject: [PATCH 2/2] 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')