From 08bb5c657f34ec7711527801196219c014422752 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 14 Jun 2021 15:29:21 +0100 Subject: [PATCH] Fix the query to get todays totals for a service. The query had a group by on notification_type and notification_status, this not only slows the query down but is wrong. The query only looked at the first result, but this query would return as many rows as different notification types and status, meaning the results do not include the correct number. Are we concerned that all status types are included. For example letters can be cancelled or have validation-failures which shouldn't be included in the daily limit check. --- app/dao/services_dao.py | 6 ++---- tests/app/dao/test_services_dao.py | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 85449f2fb..68d5ed2cc 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -437,15 +437,13 @@ def dao_fetch_todays_stats_for_service(service_id): def fetch_todays_total_message_count(service_id): + start_date = get_london_midnight_in_utc(date.today()) result = db.session.query( func.count(Notification.id).label('count') ).filter( Notification.service_id == service_id, Notification.key_type != KEY_TYPE_TEST, - func.date(Notification.created_at) == date.today() - ).group_by( - Notification.notification_type, - Notification.status, + Notification.created_at >= start_date ).first() return 0 if result is None else result.count diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index fff928206..a2a716c70 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -985,10 +985,25 @@ def test_fetch_stats_should_not_gather_notifications_older_than_7_days( def test_dao_fetch_todays_total_message_count_returns_count_for_today(notify_db_session): - notification = create_notification(template=create_template(service=create_service())) + template = create_template(service=create_service()) + notification = create_notification(template=template) + # don't include notifications earlier than today + create_notification(template=template, created_at=datetime.utcnow()-timedelta(days=2)) assert fetch_todays_total_message_count(notification.service.id) == 1 +def test_dao_fetch_todays_total_message_count_returns_count_for_all_notification_type_and_selected_service( + notify_db_session +): + service = create_service() + different_service = create_service(service_name='different service') + create_notification(template=create_template(service=service)) + create_notification(template=create_template(service=service, template_type='email')) + create_notification(template=create_template(service=service, template_type='letter')) + create_notification(template=create_template(service=different_service)) + assert fetch_todays_total_message_count(service.id) == 3 + + def test_dao_fetch_todays_total_message_count_returns_0_when_no_messages_for_today(notify_db, notify_db_session): assert fetch_todays_total_message_count(uuid.uuid4()) == 0