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 <[email protected]>

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()

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

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(