From 643ff137ccd5d3facdc3120fbe0861ea1c2b5219 Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Thu, 14 Dec 2023 09:44:13 -0500 Subject: [PATCH] Adjust start dates when fetching notification stats (#2061) * Adjust start dates when fetching notification stats - No longer subracting 1 day from the retention period limit when calculating the start_date - Changed the stats_for_today query to start pulling notifications starting from midnight UTC of the current day instead of midnight today - retention period days * Fix tests --- app/dao/fact_notification_status_dao.py | 6 +- .../dao/test_fact_notification_status_dao.py | 59 +++++++++---------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 061075609a..7c805ed6aa 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -8,7 +8,7 @@ from sqlalchemy.types import DateTime, Integer from app import db -from app.dao.date_util import utc_midnight_n_days_ago +from app.dao.date_util import get_midnight, utc_midnight_n_days_ago from app.models import ( EMAIL_TYPE, KEY_TYPE_NORMAL, @@ -239,7 +239,7 @@ def fetch_notification_status_for_service_for_day(bst_day, service_id): def fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, by_template=False, limit_days=7): - start_date = utc_midnight_n_days_ago(limit_days - 1) + start_date = utc_midnight_n_days_ago(limit_days) stats_for_7_days = db.session.query( FactNotificationStatus.notification_type.label("notification_type"), @@ -260,7 +260,7 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_ *([func.count().label("count")]), ) .filter( - Notification.created_at >= start_date, + Notification.created_at >= get_midnight(datetime.utcnow()), Notification.service_id == service_id, Notification.key_type != KEY_TYPE_TEST, ) diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index c32f0d98c4..f96fab220a 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -249,10 +249,9 @@ def test_fetch_notification_status_for_service_for_today_and_7_previous_days( email_template = create_template(service=service_1, template_type=EMAIL_TYPE) create_ft_notification_status(date(2018, 10, 29), "sms", service_1, count=10) - create_ft_notification_status(date(2018, 10, 24), "sms", service_1, count=8) create_ft_notification_status(date(2018, 10, 29), "sms", service_1, notification_status="created") + create_ft_notification_status(date(2018, 10, 24), "sms", service_1, count=8) create_ft_notification_status(date(2018, 10, 29), "email", service_1, count=3) - create_ft_notification_status(date(2018, 10, 26), "letter", service_1, count=5) save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))) save_notification(create_notification(sms_template_2, created_at=datetime(2018, 10, 31, 11, 0, 0))) @@ -272,42 +271,38 @@ def test_fetch_notification_status_for_service_for_today_and_7_previous_days( key=lambda x: (x.notification_type, x.status), ) - assert len(results) == 4 + assert len(results) == 3 assert results[0].notification_type == "email" assert results[0].status == "delivered" assert results[0].count == 4 - assert results[1].notification_type == "letter" - assert results[1].status == "delivered" - assert results[1].count == 5 + assert results[1].notification_type == "sms" + assert results[1].status == "created" + assert results[1].count == 3 assert results[2].notification_type == "sms" - assert results[2].status == "created" - assert results[2].count == 3 - - assert results[3].notification_type == "sms" - assert results[3].status == "delivered" - assert results[3].count == 11 + assert results[2].status == "delivered" + assert results[2].count == 19 @freeze_time("2018-10-31T18:00:00") # This test assumes the local timezone is EST def test_fetch_notification_status_by_template_for_service_for_today_and_7_previous_days(notify_db_session, notify_api): service_1 = create_service(service_name="service_1") - sms_template = create_template(template_name="sms Template 1", service=service_1, template_type=SMS_TYPE) - sms_template_2 = create_template(template_name="sms Template 2", service=service_1, template_type=SMS_TYPE) - email_template = create_template(service=service_1, template_type=EMAIL_TYPE) + sms_template = create_template(template_name="SMS NON-FT", service=service_1, template_type=SMS_TYPE) + sms_template_2 = create_template(template_name="SMS1 NON-FT", service=service_1, template_type=SMS_TYPE) + email_template = create_template(template_name="EMAIL NON-FT", service=service_1, template_type=EMAIL_TYPE) # create unused email template - create_template(service=service_1, template_type=EMAIL_TYPE) + create_template(template_name="UNUSED", service=service_1, template_type=EMAIL_TYPE) + # 30 sms create_ft_notification_status(date(2018, 10, 29), "sms", service_1, count=10, billable_units=20) - create_ft_notification_status(date(2018, 10, 29), "sms", service_1, count=11, billable_units=11) + create_ft_notification_status(date(2018, 10, 28), "sms", service_1, count=11, billable_units=11) create_ft_notification_status(date(2018, 10, 24), "sms", service_1, count=8) - create_ft_notification_status(date(2018, 10, 29), "sms", service_1, notification_status="created") + create_ft_notification_status(date(2018, 10, 27), "sms", service_1, notification_status="created") create_ft_notification_status(date(2018, 10, 29), "email", service_1, count=3) - create_ft_notification_status(date(2018, 10, 26), "letter", service_1, count=5) save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))) save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered")) @@ -324,13 +319,13 @@ def test_fetch_notification_status_by_template_for_service_for_today_and_7_previ ) results = fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id, by_template=True) assert [ - ("email Template Name", False, mock.ANY, "email", "delivered", 1), + ("EMAIL NON-FT", False, mock.ANY, "email", "delivered", 1), ("email Template Name", False, mock.ANY, "email", "delivered", 3), - ("letter Template Name", False, mock.ANY, "letter", "delivered", 5), - ("sms Template 1", False, mock.ANY, "sms", "created", 1), + ("SMS NON-FT", False, mock.ANY, "sms", "created", 1), ("sms Template Name", False, mock.ANY, "sms", "created", 1), - ("sms Template 1", False, mock.ANY, "sms", "delivered", 1), - ("sms Template 2", False, mock.ANY, "sms", "delivered", 1), + ("SMS NON-FT", False, mock.ANY, "sms", "delivered", 1), + ("SMS1 NON-FT", False, mock.ANY, "sms", "delivered", 1), + ("sms Template Name", False, mock.ANY, "sms", "delivered", 8), ("sms Template Name", False, mock.ANY, "sms", "delivered", 10), ("sms Template Name", False, mock.ANY, "sms", "delivered", 11), ] == sorted(results, key=lambda x: (x.notification_type, x.status, x.template_name, x.count)) @@ -1143,7 +1138,7 @@ def test_fetch_monthly_notification_statuses_per_service_for_rows_that_should_be assert len(results) == 0 -@freeze_time("2018-10-31T18:00:00") +@freeze_time("2018-11-01T18:00:00") def test_fetch_notification_status_for_service_for_today_handles_midnight_utc( notify_db_session, ): @@ -1152,13 +1147,16 @@ def test_fetch_notification_status_for_service_for_today_handles_midnight_utc( # create notifications that should not be included in today's count create_ft_notification_status(date(2018, 10, 29), "email", service_1, count=30) + save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 23, 59, 59), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 11, 59, 59), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 11, 59, 59), status="delivered")) save_notification(create_notification(email_template, created_at=datetime(2018, 10, 30, 23, 59, 59), status="delivered")) - save_notification(create_notification(email_template, created_at=datetime(2018, 10, 30, 11, 59, 59), status="delivered")) - save_notification(create_notification(email_template, created_at=datetime(2018, 10, 29, 11, 59, 59), status="delivered")) # create notifications that should be included in count - save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered")) - save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 1, 0, 0), status="delivered")) + create_ft_notification_status(date(2018, 10, 30), "email", service_1, count=5) + + save_notification(create_notification(email_template, created_at=datetime(2018, 11, 1, 13, 0, 0), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 11, 1, 1, 0, 0), status="delivered")) # checking the daily stats for this day should give us the 2 created after 12am UTC results = sorted( @@ -1173,5 +1171,4 @@ def test_fetch_notification_status_for_service_for_today_handles_midnight_utc( fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id, limit_days=2), key=lambda x: (x.notification_type, x.status), ) - - assert results[0][2] == 4 + assert results[0][2] == 7