Don't send the SMS if we have failed to save in the database

app/celery/tasks.py

@@ -4,6 +4,7 @@ from app.dao.templates_dao import get_model_templates
 from app.dao.notifications_dao import save_notification
 from app.models import Notification
 from flask import current_app
+from sqlalchemy.exc import SQLAlchemyError
 
 
 @notify_celery.task(name="send-sms")
@@ -11,20 +12,21 @@ def send_sms(service_id, notification_id, encrypted_notification):
     notification = encryption.decrypt(encrypted_notification)
     template = get_model_templates(notification['template'])
 
-    status = 'sent'
-
     try:
-        twilio_client.send_sms(notification['to'], template.content)
-    except TwilioClientException as e:
-        current_app.logger.info(e)
-        status = 'failed'
-
         notification_db_object = Notification(
             id=notification_id,
             template_id=notification['template'],
             to=notification['to'],
             service_id=service_id,
-        status=status
+            status='sent'
         )
-
         save_notification(notification_db_object)
+
+        try:
+            twilio_client.send_sms(notification['to'], template.content)
+        except TwilioClientException as e:
+            current_app.logger.debug(e)
+            save_notification(notification_db_object, {"status": "failed"})
+
+    except SQLAlchemyError as e:
+        current_app.logger.debug(e)

app/encryption.py

@@ -4,14 +4,12 @@ from itsdangerous import URLSafeSerializer
 
 
 class Encryption:
-
     def init_app(self, app):
         self.serializer = URLSafeSerializer(app.config.get('SECRET_KEY'))
         self.salt = app.config.get('DANGEROUS_SALT')
 
     def encrypt(self, thing_to_encrypt):
-        return self.serializer.dumps(thing_to_encrypt, salt=
-        self.salt)
+        return self.serializer.dumps(thing_to_encrypt, salt=self.salt)
 
     def decrypt(self, thing_to_decrypt):
         return self.serializer.loads(thing_to_decrypt, salt=self.salt)

tests/app/celery/test_tasks.py

@@ -1,8 +1,11 @@
 import uuid
+import pytest
 from app.celery.tasks import send_sms
 from app import twilio_client
 from app.clients.sms.twilio import TwilioClientException
 from app.dao import notifications_dao
+from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.orm.exc import NoResultFound
 
 
 def test_should_send_template_to_correct_sms_provider_and_persist(sample_template, mocker):
@@ -58,6 +61,7 @@ def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker):
     }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.twilio_client.send_sms', side_effect=TwilioClientException())
+    mocker.patch('app.db.session.add', side_effect=SQLAlchemyError())
 
     notification_id = uuid.uuid4()
 
@@ -66,9 +70,7 @@ def test_should_not_send_sms_if_db_peristance_failed(sample_template, mocker):
         notification_id,
         "encrypted-in-reality")
 
-    twilio_client.send_sms.assert_called_once_with("+441234123123", sample_template.content)
-    persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id)
-    assert persisted_notification.id == notification_id
-    assert persisted_notification.to == '+441234123123'
-    assert persisted_notification.template_id == sample_template.id
-    assert persisted_notification.status == 'failed'
+    twilio_client.send_sms.assert_not_called()
+    with pytest.raises(NoResultFound) as e:
+        notifications_dao.get_notification(sample_template.service_id, notification_id)
+    assert 'No row was found for one' in str(e.value)

tests/app/conftest.py

@@ -9,6 +9,7 @@ from app.dao.jobs_dao import save_job
 from app.dao.notifications_dao import save_notification
 import uuid
 
+
 @pytest.fixture(scope='function')
 def service_factory(notify_db, notify_db_session):
     class ServiceFactory(object):