diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 46468e9fd..ecce6fb1c 100644 --- a/app/celery/tasks.py +++ b/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( diff --git a/app/models.py b/app/models.py index 4f22108c4..569125374 100644 --- a/app/models.py +++ b/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) diff --git a/migrations/versions/0017_add_failure_types.py b/migrations/versions/0017_add_failure_types.py new file mode 100644 index 000000000..f7d55145c --- /dev/null +++ b/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) \ No newline at end of file diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index ec537ceba..8453e0a85 100644 --- a/tests/app/celery/test_tasks.py +++ b/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