diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 827ed3f1d..89f6333b1 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -23,6 +23,7 @@ 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.clients.document_download import DocumentDownloadError from app.config import QueueNames, TaskNames +from app.dao.dao_utils import transaction from app.dao.templates_dao import get_precompiled_letter_template from app.letters.utils import upload_letter_pdf from app.models import ( @@ -412,12 +413,14 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, service, t except ValueError: raise BadRequestError(message='Cannot decode letter content (invalid base64 encoding)', status_code=400) - notification = create_letter_notification(letter_data=letter_data, - service=service, - template=template, - api_key=api_key, - status=status, - reply_to_text=reply_to_text) + with transaction(): + notification = create_letter_notification(letter_data=letter_data, + service=service, + template=template, + api_key=api_key, + status=status, + reply_to_text=reply_to_text) + filename = upload_letter_pdf(notification, letter_content, precompiled=True) resp = { 'id': notification.id, @@ -425,8 +428,6 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, service, t 'postage': notification.postage } - filename = upload_letter_pdf(notification, letter_content, precompiled=True) - current_app.logger.info('Calling task scan-file for {}'.format(filename)) # call task to add the filename to anti virus queue diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index bb733621f..fb37b28be 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -20,6 +20,9 @@ from app.models import ( Job, Notification, ) +from app.notifications.process_letter_notifications import ( + create_letter_notification, +) from app.schema_validation import validate from app.v2.errors import RateLimitError from app.v2.notifications.notification_schemas import post_letter_response @@ -673,6 +676,31 @@ def test_post_precompiled_letter_notification_returns_201( assert resp_json == {'id': str(notification.id), 'reference': 'letter-reference', 'postage': expected_postage} +def test_post_precompiled_letter_notification_if_s3_upload_fails_notification_is_not_persisted( + client, notify_user, mocker +): + sample_service = create_service(service_permissions=['letter']) + persist_letter_mock = mocker.patch('app.v2.notifications.post_notifications.create_letter_notification', + side_effect=create_letter_notification) + s3mock = mocker.patch('app.v2.notifications.post_notifications.upload_letter_pdf', side_effect=Exception()) + mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task') + data = { + "reference": "letter-reference", + "content": "bGV0dGVyLWNvbnRlbnQ=" + } + + auth_header = create_authorization_header(service_id=sample_service.id) + with pytest.raises(expected_exception=Exception): + client.post( + path="v2/notifications/letter", + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert s3mock.called + assert persist_letter_mock.called + assert Notification.query.count() == 0 + + def test_post_letter_notification_throws_error_for_invalid_postage(client, notify_user, mocker): sample_service = create_service(service_permissions=['letter']) data = {