Revert "Revert "Match the 5AM->5AM time window used to populate ft_no…

…tifications (#2069)" (#2070)" (#2074)

Match the 5AM->5AM time window used to populate ft_notifications

Co-authored-by: Jumana B <[email protected]>

app/dao/date_util.py

@@ -83,6 +83,22 @@ def get_midnight(datetime: datetime) -> datetime:
     return datetime.replace(hour=0, minute=0, second=0, microsecond=0)
 
 
+def tz_aware_utc_now() -> datetime:
+    """
+    Returns a localized, EST/EDT timezone aware, UTC now datetime.
+    Call dst() on the returned object to determine daylight savings status.
+    """
+    return pytz.utc.localize(datetime.utcnow())
+
+
+def tz_aware_midnight_n_days_ago(days_ago: int = 1) -> datetime:
+    """
+    Returns an EST/EDT aware UTC midnight date a number of days ago.
+    """
+    est = pytz.timezone("US/Eastern")
+    return est.localize(tz_aware_utc_now().replace(tzinfo=None) - timedelta(days=days_ago))
+
+
 def utc_midnight_n_days_ago(number_of_days):
     """
     Returns utc midnight a number of days ago.

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 get_midnight, utc_midnight_n_days_ago
+from app.dao.date_util import tz_aware_midnight_n_days_ago, utc_midnight_n_days_ago
 from app.models import (
     EMAIL_TYPE,
     KEY_TYPE_NORMAL,
@@ -239,7 +239,13 @@ 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)
+    ft_start_date = utc_midnight_n_days_ago(limit_days)
+
+    # The nightly task that populates ft_notification_status counts collects notifications from
+    # 5AM the day before to 5AM of the current day. So we need to match that timeframe when
+    # we fetch notifications for the current day.
+    start = (tz_aware_midnight_n_days_ago(1) + timedelta(hours=5)).replace(minute=0, second=0, microsecond=0)
+    end = (tz_aware_midnight_n_days_ago(0) + timedelta(hours=5)).replace(minute=0, second=0, microsecond=0)
 
     stats_for_7_days = db.session.query(
         FactNotificationStatus.notification_type.label("notification_type"),
@@ -248,7 +254,7 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_
         *([FactNotificationStatus.notification_count.label("count")]),
     ).filter(
         FactNotificationStatus.service_id == service_id,
-        FactNotificationStatus.bst_date >= start_date,
+        FactNotificationStatus.bst_date >= ft_start_date,
         FactNotificationStatus.key_type != KEY_TYPE_TEST,
     )
 
@@ -260,7 +266,8 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_
             *([func.count().label("count")]),
         )
         .filter(
-            Notification.created_at >= get_midnight(datetime.utcnow()),
+            Notification.created_at >= start,
+            Notification.created_at <= end,
             Notification.service_id == service_id,
             Notification.key_type != KEY_TYPE_TEST,
         )

tests/app/dao/test_fact_notification_status_dao.py

@@ -1138,6 +1138,10 @@ def test_fetch_monthly_notification_statuses_per_service_for_rows_that_should_be
     assert len(results) == 0
 
 
+# Freezegun is currently unable of handling non-timezone naive dates:
+# https://github.com/spulec/freezegun/issues/89 : https://github.com/spulec/freezegun/issues/487
+# So while the timeframe boundaries we're testing here are 5AM to 5AM UTC across 2 days, because the start/end dates
+# are timezone aware our boundaries for the purpose of this test are 23h to 23h.
 @freeze_time("2018-11-01T18:00:00")
 def test_fetch_notification_status_for_service_for_today_handles_midnight_utc(
     notify_db_session,
@@ -1147,28 +1151,28 @@ 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, 0, 0, 0), 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, 31, 23, 59, 59), status="delivered"))
 
     # create notifications that should be included in count
+    create_ft_notification_status(date(2018, 10, 31), "email", service_1, count=5)
     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"))
+    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, 6, 0, 0), status="delivered"))
+    save_notification(create_notification(email_template, created_at=datetime(2018, 11, 1, 22, 59, 59), status="delivered"))
 
     # checking the daily stats for this day should give us the 2 created after 12am UTC
     results = sorted(
         fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id, limit_days=1),
         key=lambda x: (x.notification_type, x.status),
     )
-
-    assert results[0][2] == 2
+    assert results[0][2] == 6
 
     # checking the daily stats for the last 2 days should give us the 2 created after 12am UTC and the 1 from the day before
     results = sorted(
         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] == 7
+    assert results[0][2] == 11