From 49db4e81d538bda8322294c77949de4b04f2a0d7 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 17 May 2016 11:46:28 +0100 Subject: [PATCH 1/2] Add new notification status types for technical_failure, temporary_failure, permanent_failure --- app/models.py | 5 +- migrations/versions/0017_add_failure_types.py | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/0017_add_failure_types.py diff --git a/app/models.py b/app/models.py index 497153a05..90885d57c 100644 --- a/app/models.py +++ b/app/models.py @@ -304,7 +304,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): @@ -339,7 +340,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..8f8ba241b --- /dev/null +++ b/migrations/versions/0017_add_failure_types.py @@ -0,0 +1,47 @@ +"""empty message + +Revision ID: 0017_add_failure_types +Revises: 0015_fix_template_data +Create Date: 2016-05-17 11:23:36.881219 + +""" + +# revision identifiers, used by Alembic. +revision = '0017_add_failure_types' +down_revision = '0015_fix_template_data' + +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 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 From 191a79f27b1c47b72fa7f0b1fec31dfa7c1bfe03 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 17 May 2016 13:06:08 +0100 Subject: [PATCH 2/2] Add new failure status for notifications. --- app/celery/tasks.py | 7 +++++-- app/models.py | 2 +- migrations/versions/0017_add_failure_types.py | 8 +++++--- tests/app/celery/test_tasks.py | 9 ++++----- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 993bdea0f..078c75199 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, @@ -264,7 +267,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( @@ -334,7 +337,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 27c1a8276..569125374 100644 --- a/app/models.py +++ b/app/models.py @@ -306,7 +306,7 @@ class VerifyCode(db.Model): NOTIFICATION_STATUS_TYPES = ['sending', 'delivered', 'failed', - 'technical_failure', 'temporary_failure', 'permanent_failure'] + 'technical-failure', 'temporary-failure', 'permanent-failure'] class Notification(db.Model): diff --git a/migrations/versions/0017_add_failure_types.py b/migrations/versions/0017_add_failure_types.py index 8f8ba241b..f7d55145c 100644 --- a/migrations/versions/0017_add_failure_types.py +++ b/migrations/versions/0017_add_failure_types.py @@ -1,14 +1,14 @@ """empty message Revision ID: 0017_add_failure_types -Revises: 0015_fix_template_data +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 = '0015_fix_template_data' +down_revision = '0016_reply_to_email' from alembic import op import sqlalchemy as sa @@ -20,7 +20,7 @@ from sqlalchemy.dialects import postgresql def upgrade(): status_type = sa.Enum('sending', 'delivered', 'failed', - 'technical_failure', 'temporary_failure', 'permanent_failure', + '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)) @@ -38,6 +38,8 @@ def downgrade(): 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') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 0d92dff29..749f8ca61 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