Merge pull request #3218 from alphagov/precomplied-letter-transation

Introduce transaction for precompiled letters

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,
+    with transaction():
-                                              service=service,
+        notification = create_letter_notification(letter_data=letter_data,
-                                              template=template,
+                                                  service=service,
-                                              api_key=api_key,
+                                                  template=template,
-                                              status=status,
+                                                  api_key=api_key,
-                                              reply_to_text=reply_to_text)
+                                                  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

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 = {