From 1dc084be5441a6d42035dba2dcb02504270a95e1 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 2 Apr 2019 15:15:07 +0100 Subject: [PATCH] fix nightly ft stats tables task to respect BST the create_nightly_notification_status task runs at 00:30am UK time, however this means that in summer datetime.today() will return the wrong date as the server (which runs on UTC) will run the task at 23:30 (populating the wrong row in the table). fix this to use nice tz aware functions --- app/celery/reporting_tasks.py | 9 +++--- app/dao/fact_billing_dao.py | 2 +- app/dao/fact_notification_status_dao.py | 4 +-- tests/app/celery/test_reporting_tasks.py | 32 ++++++++++++++++++- .../dao/test_fact_notification_status_dao.py | 6 ++-- 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index 80c5d1bc0..d8aaa3ef3 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -10,6 +10,7 @@ from app.dao.fact_billing_dao import ( update_fact_billing ) from app.dao.fact_notification_status_dao import fetch_notification_status_for_day, update_fact_notification_status +from app.utils import convert_utc_to_bst @notify_celery.task(name="create-nightly-billing") @@ -19,10 +20,10 @@ def create_nightly_billing(day_start=None): # day_start is a datetime.date() object. e.g. # up to 10 days of data counting back from day_start is consolidated if day_start is None: - day_start = datetime.today() - timedelta(days=1) + day_start = convert_utc_to_bst(datetime.utcnow()).date() - timedelta(days=1) else: # When calling the task its a string in the format of "YYYY-MM-DD" - day_start = datetime.strptime(day_start, "%Y-%m-%d") + day_start = datetime.strptime(day_start, "%Y-%m-%d").date() for i in range(0, 10): process_day = day_start - timedelta(days=i) @@ -42,10 +43,10 @@ def create_nightly_notification_status(day_start=None): # day_start is a datetime.date() object. e.g. # 4 days of data counting back from day_start is consolidated if day_start is None: - day_start = datetime.today() - timedelta(days=1) + day_start = convert_utc_to_bst(datetime.utcnow()).date() - timedelta(days=1) else: # When calling the task its a string in the format of "YYYY-MM-DD" - day_start = datetime.strptime(day_start, "%Y-%m-%d") + day_start = datetime.strptime(day_start, "%Y-%m-%d").date() for i in range(0, 4): process_day = day_start - timedelta(days=i) diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index bdc2011a6..44b1786db 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -287,7 +287,7 @@ def update_fact_billing(data, process_day): def create_billing_record(data, rate, process_day): billing_record = FactBilling( - bst_date=process_day.date(), + bst_date=process_day, template_id=data.template_id, service_id=data.service_id, notification_type=data.notification_type, diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 60b4bb92c..df509ecd9 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -54,11 +54,11 @@ def fetch_notification_status_for_day(process_day, service_id=None): def update_fact_notification_status(data, process_day): table = FactNotificationStatus.__table__ FactNotificationStatus.query.filter( - FactNotificationStatus.bst_date == process_day.date() + FactNotificationStatus.bst_date == process_day ).delete() for row in data: stmt = insert(table).values( - bst_date=process_day.date(), + bst_date=process_day, template_id=row.template_id, service_id=row.service_id, job_id=row.job_id, diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index ade175db2..7784dcf67 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -357,7 +357,7 @@ def test_get_rate_for_sms_and_email(notify_db_session): assert rate == Decimal(0) -@freeze_time('2018-03-27T03:30:00') +@freeze_time('2018-03-26T23:30:00') # summer time starts on 2018-03-25 def test_create_nightly_billing_use_BST( sample_service, @@ -486,3 +486,33 @@ def test_create_nightly_notification_status(notify_db_session): assert str(new_data[0].bst_date) == datetime.strftime(datetime.utcnow() - timedelta(days=4), "%Y-%m-%d") 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") + + +# 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): + 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)) + create_notification(sample_template, status='created', created_at=datetime(2019, 3, 31, 23, 0)) + + create_notification(sample_template, status='temporary-failure', created_at=datetime(2019, 3, 31, 22, 59)) + + # we create records for last four days + create_notification(sample_template, status='sending', created_at=datetime(2019, 3, 29, 0, 0)) + + create_notification(sample_template, status='delivered', created_at=datetime(2019, 3, 28, 23, 59)) # too old + + create_nightly_notification_status() + + noti_status = FactNotificationStatus.query.order_by(FactNotificationStatus.bst_date).all() + assert len(noti_status) == 3 + + assert noti_status[0].bst_date == date(2019, 3, 29) + assert noti_status[0].notification_status == 'sending' + + assert noti_status[1].bst_date == date(2019, 3, 31) + assert noti_status[1].notification_status == 'temporary-failure' + + assert noti_status[2].bst_date == date(2019, 4, 1) + assert noti_status[2].notification_status == 'created' diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 5239a213c..3406c7ec3 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -38,7 +38,7 @@ def test_update_fact_notification_status(notify_db_session): process_day = datetime.utcnow() data = fetch_notification_status_for_day(process_day=process_day) - update_fact_notification_status(data=data, process_day=process_day) + update_fact_notification_status(data=data, process_day=process_day.date()) new_fact_data = FactNotificationStatus.query.order_by(FactNotificationStatus.bst_date, FactNotificationStatus.notification_type @@ -77,7 +77,7 @@ def test__update_fact_notification_status_updates_row(notify_db_session): process_day = datetime.utcnow() data = fetch_notification_status_for_day(process_day=process_day) - update_fact_notification_status(data=data, process_day=process_day) + update_fact_notification_status(data=data, process_day=process_day.date()) new_fact_data = FactNotificationStatus.query.order_by(FactNotificationStatus.bst_date, FactNotificationStatus.notification_type @@ -88,7 +88,7 @@ def test__update_fact_notification_status_updates_row(notify_db_session): create_notification(template=first_template, status='delivered') data = fetch_notification_status_for_day(process_day=process_day) - update_fact_notification_status(data=data, process_day=process_day) + update_fact_notification_status(data=data, process_day=process_day.date()) updated_fact_data = FactNotificationStatus.query.order_by(FactNotificationStatus.bst_date, FactNotificationStatus.notification_type