Skip to content

Commit

Permalink
Adjust start dates when fetching notification stats (#2061)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@whabanks
whabanks authored Dec 14, 2023
1 parent f5cf93e commit 643ff13
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 34 deletions.
6 changes: 3 additions & 3 deletions app/dao/fact_notification_status_dao.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
Expand All @@ -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,
)
Expand Down
59 changes: 28 additions & 31 deletions tests/app/dao/test_fact_notification_status_dao.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand All @@ -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"))
Expand All @@ -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))
Expand Down Expand Up @@ -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,
):
Expand All @@ -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(
Expand All @@ -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

0 comments on commit 643ff13

Please sign in to comment.