From 58f24a0a83089c4093eaa732b4314dcb03ea0df6 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 14 Aug 2019 17:38:09 +0100 Subject: [PATCH] trigger nightly delete tasks from the create notification status task the nightly tasks need to run after the create nightly notification status task - so that test notifications are still there to record stats for, and to stop the risk of deleting notificaitons part-way through recording stats for them. --- app/celery/reporting_tasks.py | 11 +++++++++++ app/config.py | 15 --------------- tests/app/celery/test_reporting_tasks.py | 17 +++++++++++++++-- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index 07e0a66c6..04c3c48ec 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -5,12 +5,18 @@ from notifications_utils.statsd_decorators import statsd from notifications_utils.timezones import convert_utc_to_bst from app import notify_celery +from app.config import QueueNames from app.cronitor import cronitor from app.dao.fact_billing_dao import ( fetch_billing_data_for_day, update_fact_billing ) from app.dao.fact_notification_status_dao import fetch_notification_status_for_day, update_fact_notification_status +from app.celery.nightly_tasks import ( + delete_sms_notifications_older_than_retention, + delete_email_notifications_older_than_retention, + delete_letter_notifications_older_than_retention +) @notify_celery.task(name="create-nightly-billing") @@ -59,3 +65,8 @@ def create_nightly_notification_status(day_start=None): len(transit_data), process_day ) ) + + # delete jobs need to happen after nightly notification status is recorded to avoid conflict between the two tasks + delete_email_notifications_older_than_retention.apply_async(queue=QueueNames.PERIODIC) + delete_sms_notifications_older_than_retention.apply_async(queue=QueueNames.PERIODIC) + delete_letter_notifications_older_than_retention.apply_async(queue=QueueNames.PERIODIC) diff --git a/app/config.py b/app/config.py index 46b8c48c0..b4eaabb11 100644 --- a/app/config.py +++ b/app/config.py @@ -222,21 +222,6 @@ class Config(object): 'schedule': crontab(hour=0, minute=30), # after 'timeout-sending-notifications' 'options': {'queue': QueueNames.PERIODIC} }, - 'delete-sms-notifications': { - 'task': 'delete-sms-notifications', - 'schedule': crontab(hour=0, minute=45), # after 'create-nightly-notification-status' - 'options': {'queue': QueueNames.PERIODIC} - }, - 'delete-email-notifications': { - 'task': 'delete-email-notifications', - 'schedule': crontab(hour=1, minute=0), # after 'create-nightly-notification-status' - 'options': {'queue': QueueNames.PERIODIC} - }, - 'delete-letter-notifications': { - 'task': 'delete-letter-notifications', - 'schedule': crontab(hour=1, minute=20), # after 'create-nightly-notification-status' - 'options': {'queue': QueueNames.PERIODIC} - }, 'delete-inbound-sms': { 'task': 'delete-inbound-sms', 'schedule': crontab(hour=1, minute=40), diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index d4d11773c..6da7397d6 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -429,7 +429,13 @@ def test_create_nightly_billing_update_when_record_exists( assert records[0].updated_at -def test_create_nightly_notification_status(notify_db_session): +def test_create_nightly_notification_status(notify_db_session, mocker): + mocks = [ + mocker.patch('app.celery.reporting_tasks.delete_email_notifications_older_than_retention'), + mocker.patch('app.celery.reporting_tasks.delete_sms_notifications_older_than_retention'), + mocker.patch('app.celery.reporting_tasks.delete_letter_notifications_older_than_retention'), + ] + first_service = create_service(service_name='First Service') first_template = create_template(service=first_service) second_service = create_service(service_name='second Service') @@ -471,10 +477,17 @@ def test_create_nightly_notification_status(notify_db_session): assert str(new_data[3].bst_date) == datetime.strftime(datetime.utcnow() - timedelta(days=2), "%Y-%m-%d") assert str(new_data[6].bst_date) == datetime.strftime(datetime.utcnow() - timedelta(days=1), "%Y-%m-%d") + for mock in mocks: + mock.apply_async.assert_called_once_with(queue='periodic-tasks') + # the job runs at 12:30am London time. 04/01 is in BST. @freeze_time('2019-04-01T23:30') -def test_create_nightly_notification_status_respects_bst(sample_template): +def test_create_nightly_notification_status_respects_bst(sample_template, mocker): + mocker.patch('app.celery.reporting_tasks.delete_email_notifications_older_than_retention') + mocker.patch('app.celery.reporting_tasks.delete_sms_notifications_older_than_retention') + mocker.patch('app.celery.reporting_tasks.delete_letter_notifications_older_than_retention') + create_notification(sample_template, status='delivered', created_at=datetime(2019, 4, 1, 23, 0)) # too new create_notification(sample_template, status='created', created_at=datetime(2019, 4, 1, 22, 59))