Fix getting service IDs for status aggregation

Addresses [1].

Previously the query would always use UTC midnight, even after we
had switched to BST (+1h). We store timestamps as naive UTC in our
DB - without a timezone - but we want the query to work in terms
of GMT / BST so we adjust for that - BST midnight is 11PM in UTC.

[1]: https://github.com/alphagov/notifications-api/pull/3437#discussion_r791998690
This commit is contained in:
Ben Thorner
2022-02-10 10:37:32 +00:00
parent 6e8f121548
commit 966c4db8c6
2 changed files with 32 additions and 4 deletions

View File

@@ -46,7 +46,11 @@ from app.models import (
NotificationHistory,
ProviderDetails,
)
from app.utils import escape_special_characters, midnight_n_days_ago
from app.utils import (
escape_special_characters,
get_london_midnight_in_utc,
midnight_n_days_ago,
)
def dao_get_last_date_template_was_used(template_id, service_id):
@@ -797,6 +801,9 @@ def get_service_ids_with_notifications_before(notification_type, timestamp):
def get_service_ids_with_notifications_on_date(notification_type, date):
start_date = get_london_midnight_in_utc(date)
end_date = get_london_midnight_in_utc(date + timedelta(days=1))
return {
row.service_id
for row in db.session.query(
@@ -804,7 +811,7 @@ def get_service_ids_with_notifications_on_date(notification_type, date):
).filter(
Notification.notification_type == notification_type,
# using >= + < is much more efficient than date(created_at)
Notification.created_at >= date,
Notification.created_at < date + timedelta(days=1)
Notification.created_at >= start_date,
Notification.created_at < end_date,
).distinct()
}

View File

@@ -1,5 +1,5 @@
import uuid
from datetime import datetime, timedelta
from datetime import date, datetime, timedelta
from functools import partial
import pytest
@@ -24,6 +24,7 @@ from app.dao.notifications_dao import (
get_notification_with_personalisation,
get_notifications_for_job,
get_notifications_for_service,
get_service_ids_with_notifications_on_date,
is_delivery_slow_for_providers,
notifications_not_yet_sent,
update_notification_status_by_id,
@@ -41,6 +42,7 @@ from app.models import (
NOTIFICATION_STATUS_TYPES,
NOTIFICATION_STATUS_TYPES_FAILED,
NOTIFICATION_TEMPORARY_FAILURE,
SMS_TYPE,
Job,
Notification,
NotificationHistory,
@@ -1721,3 +1723,22 @@ def test_dao_get_letters_and_sheets_volume_by_postage(notify_db_session):
for result in results:
assert result._asdict() in expected_results
@pytest.mark.parametrize('created_at_utc,date_to_check,expected_count', [
# Clocks change on the 27th of March 2022, so the query needs to look at the
# time range 00:00 - 23:00 (UTC) thereafter.
('2022-03-27T00:30', date(2022, 3, 27), 1), # 27/03 00:30 GMT
('2022-03-27T22:30', date(2022, 3, 27), 1), # 27/03 23:30 BST
('2022-03-27T23:30', date(2022, 3, 27), 0), # 28/03 00:30 BST
('2022-03-26T23:30', date(2022, 3, 26), 1), # 26/03 23:30 GMT
])
def test_get_service_ids_with_notifications_on_date_respects_gmt_bst(
sample_template,
created_at_utc,
date_to_check,
expected_count
):
create_notification(template=sample_template, created_at=created_at_utc)
service_ids = get_service_ids_with_notifications_on_date(SMS_TYPE, date_to_check)
assert len(service_ids) == expected_count