Merge pull request #318 from alphagov/tech-failures

New notification status types

app/celery/tasks.py

@@ -86,6 +86,9 @@ def delete_failed_notifications():
     try:
         start = datetime.utcnow()
         deleted = delete_notifications_created_more_than_a_week_ago('failed')
+        deleted += delete_notifications_created_more_than_a_week_ago('technical-failure')
+        deleted += delete_notifications_created_more_than_a_week_ago('temporary-failure')
+        deleted += delete_notifications_created_more_than_a_week_ago('permanent-failure')
         current_app.logger.info(
             "Delete job started {} finished {} deleted {} failed notifications".format(
                 start,
@@ -268,7 +271,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
                 "SMS notification {} failed".format(notification_id)
             )
             current_app.logger.exception(e)
-            notification_db_object.status = 'failed'
+            notification_db_object.status = 'technical-failure'
 
         dao_update_notification(notification_db_object)
         current_app.logger.info(
@@ -338,7 +341,7 @@ def send_email(service_id, notification_id, from_address, encrypted_notification
 
         except EmailClientException as e:
             current_app.logger.exception(e)
-            notification_db_object.status = 'failed'
+            notification_db_object.status = 'technical-failure'
             dao_update_notification(notification_db_object)
 
         current_app.logger.info(

app/models.py

@@ -305,7 +305,8 @@ class VerifyCode(db.Model):
         return check_hash(cde, self._code)
 
 
-NOTIFICATION_STATUS_TYPES = ['sending', 'delivered', 'failed']
+NOTIFICATION_STATUS_TYPES = ['sending', 'delivered', 'failed',
+                             'technical-failure', 'temporary-failure', 'permanent-failure']
 
 
 class Notification(db.Model):
@@ -340,7 +341,7 @@ class Notification(db.Model):
         nullable=True,
         onupdate=datetime.datetime.utcnow)
     status = db.Column(
-        db.Enum(*NOTIFICATION_STATUS_TYPES, name='notification_status_types'), nullable=False, default='sending')
+        db.Enum(*NOTIFICATION_STATUS_TYPES, name='notification_status_type'), nullable=False, default='sending')
     reference = db.Column(db.String, nullable=True, index=True)
 
 

migrations/versions/0017_add_failure_types.py

@@ -0,0 +1,49 @@
+"""empty message
+
+Revision ID: 0017_add_failure_types
+Revises: 0016_reply_to_email
+Create Date: 2016-05-17 11:23:36.881219
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0017_add_failure_types'
+down_revision = '0016_reply_to_email'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+def upgrade():
+    status_type = sa.Enum('sending', 'delivered', 'failed',
+                          'technical-failure', 'temporary-failure', 'permanent-failure',
+                          name='notification_status_type')
+    status_type.create(op.get_bind())
+    op.add_column('notifications', sa.Column('new_status', status_type, nullable=True))
+    op.execute('update notifications set new_status = CAST(CAST(status as text) as notification_status_type)')
+    op.alter_column('notifications', 'status', new_column_name='old_status')
+    op.alter_column('notifications', 'new_status', new_column_name='status')
+    op.drop_column('notifications', 'old_status')
+    op.get_bind()
+    op.execute('DROP TYPE notification_status_types')
+    op.alter_column('notifications', 'status', nullable=False)
+
+
+def downgrade():
+    status_type = sa.Enum('sending', 'delivered', 'failed',
+                          name='notification_status_types')
+    status_type.create(op.get_bind())
+    op.add_column('notifications', sa.Column('old_status', status_type, nullable=True))
+
+    op.execute("update notifications set status = 'failed' where status in ('technical-failure', 'temporary-failure', 'permanent-failure')")
+    op.execute('update notifications set old_status = CAST(CAST(status as text) as notification_status_types)')
+    op.alter_column('notifications', 'status', new_column_name='new_status')
+    op.alter_column('notifications', 'old_status', new_column_name='status')
+    op.drop_column('notifications', 'new_status')
+    op.get_bind()
+    op.execute('DROP TYPE notification_status_type')
+    op.alter_column('notifications', 'status', nullable=False)

tests/app/celery/test_tasks.py

@@ -84,10 +84,9 @@ def test_should_call_delete_notifications_more_than_week_in_task(notify_api, moc
 
 
 def test_should_call_delete_notifications_more_than_week_in_task(notify_api, mocker):
-    mocked = mocker.patch('app.celery.tasks.delete_notifications_created_more_than_a_week_ago')
+    mocker.patch('app.celery.tasks.delete_notifications_created_more_than_a_week_ago')
     delete_failed_notifications()
-    mocked.assert_called_with('failed')
-    assert tasks.delete_notifications_created_more_than_a_week_ago.call_count == 1
+    assert tasks.delete_notifications_created_more_than_a_week_ago.call_count == 4
 
 
 def test_should_call_delete_codes_on_delete_verify_codes_task(notify_api, mocker):
@@ -788,7 +787,7 @@ def test_should_persist_notification_as_failed_if_sms_client_fails(sample_templa
     assert persisted_notification.to == '+447234123123'
     assert persisted_notification.template_id == sample_template.id
     assert persisted_notification.template_version == sample_template.version
-    assert persisted_notification.status == 'failed'
+    assert persisted_notification.status == 'technical-failure'
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_at > now
     assert persisted_notification.sent_by == 'mmg'
@@ -823,7 +822,7 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai
     assert persisted_notification.to == 'my_email@my_email.com'
     assert persisted_notification.template_id == sample_email_template.id
     assert persisted_notification.template_version == sample_email_template.version
-    assert persisted_notification.status == 'failed'
+    assert persisted_notification.status == 'technical-failure'
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_by == 'ses'
     assert persisted_notification.sent_at > now