From af908690a155a78cf0acf848ef2ddab14f3402cb Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 27 Sep 2024 09:46:10 -0300 Subject: [PATCH] Update `get_notifications_for_service` (#2307) * fix(get_notifications_for_service): update query to use same start date as the rest * tests: fix and new test * chore: formatting * chore: formatting! --------- Co-authored-by: Jumana B --- app/dao/notifications_dao.py | 7 ++----- .../notification_dao/test_notification_dao.py | 18 +++++++++++++++--- tests/app/service/test_rest.py | 4 ++-- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 3c78e02703..818f99fbc4 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -25,10 +25,7 @@ from app import create_uuid, db, signer_personalisation from app.dao.dao_utils import transactional -from app.dao.date_util import ( - get_query_date_based_on_retention_period, - utc_midnight_n_days_ago, -) +from app.dao.date_util import get_query_date_based_on_retention_period from app.errors import InvalidRequest from app.models import ( EMAIL_TYPE, @@ -332,7 +329,7 @@ def get_notifications_for_service( filters = [Notification.service_id == service_id] if limit_days is not None: - filters.append(Notification.created_at >= utc_midnight_n_days_ago(limit_days)) + filters.append(Notification.created_at > get_query_date_based_on_retention_period(limit_days)) if older_than is not None: older_than_created_at = db.session.query(Notification.created_at).filter(Notification.id == older_than).as_scalar() diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 145915037e..7bdc6a3488 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -636,11 +636,10 @@ def test_update_notification_sets_status(sample_notification): @freeze_time("2016-01-10") -# This test assumes the local timezone is EST def test_should_limit_notifications_return_by_day_limit_plus_one(sample_template): assert len(Notification.query.all()) == 0 - # create one notification a day between 1st and 9th + # create one notification a day between 1st and 10th for i in range(1, 11): past_date = "2016-01-{0:02d} 12:00:00".format(i) with freeze_time(past_date): @@ -653,7 +652,7 @@ def test_should_limit_notifications_return_by_day_limit_plus_one(sample_template assert len(all_notifications) == 10 all_notifications = get_notifications_for_service(sample_template.service_id, limit_days=1).items - assert len(all_notifications) == 2 + assert len(all_notifications) == 1 def test_creating_notification_does_not_add_notification_history(sample_template): @@ -1894,3 +1893,16 @@ def test_resign_notifications_unsafe_resigns_with_new_key(self, sample_template_ notification = Notification.query.get(initial_notification.id) assert notification.personalisation == personalisation # unsigned value is the same assert notification._personalisation != _personalisation # signature is different + + +@freeze_time("2024-09-25 12:25:00") +def test_get_notifications_for_service(sample_template): + # create notifications for the past 10 days + for i in range(1, 11): + save_notification( + create_notification(sample_template, client_reference="xyz", created_at=datetime(2024, 9, 26 - i, 23, 59, 59)) + ) + + # ensure as we increase limit_days by 1, we get 1 more notification in the total each time + for i in range(1, 11): + assert len(get_notifications_for_service(sample_template.service_id, limit_days=i).items) == i diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 1ac8d4da06..dc35bbbbed 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1721,13 +1721,13 @@ def test_get_notifications_for_service_gets_data_from_correct_timeframe( for i in range(retention_period): for j in range(24): save_notification( - create_notification(email_template, created_at=datetime(2018, 11, 13 + i, j, 0, 0), status="delivered") + create_notification(email_template, created_at=datetime(2018, 11, 14 + i, j, 0, 0), status="delivered") ) resp = admin_request.get( "service.get_all_notifications_for_service", service_id=email_template.service_id, limit_days=7, page_size=1 ) - assert resp["total"] == expected_count_of_notifications + assert resp["total"] == expected_count_of_notifications # @pytest.mark.parametrize(