diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index e0d39e87f..3f3978f7d 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -84,11 +84,15 @@ def dao_get_template_usage(service_id, day): @statsd(namespace="dao") -def dao_get_last_template_usage(template_id, template_type): +def dao_get_last_template_usage(template_id, template_type, service_id): + # By adding the service_id to the filter the performance of the query is greatly improved. + # Using a max(Notification.created_at) is better than order by and limit one. + # But the effort to change the endpoint to return a datetime only is more than the gain. return Notification.query.filter( Notification.template_id == template_id, Notification.key_type != KEY_TYPE_TEST, - Notification.notification_type == template_type + Notification.notification_type == template_type, + Notification.service_id == service_id ).order_by( desc(Notification.created_at) ).first() diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index 5c422998f..d5ea53c23 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -47,7 +47,7 @@ def get_template_statistics_for_template_id(service_id, template_id): template = dao_get_template_by_id_and_service_id(template_id, service_id) data = None - notification = dao_get_last_template_usage(template_id, template.template_type) + notification = dao_get_last_template_usage(template_id, template.template_type, template.service_id) if notification: data = notification_with_template_schema.dump(notification).data diff --git a/tests/app/dao/notification_dao/test_notification_dao_template_usage.py b/tests/app/dao/notification_dao/test_notification_dao_template_usage.py index b8ae644c5..76800694f 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_template_usage.py +++ b/tests/app/dao/notification_dao/test_notification_dao_template_usage.py @@ -21,7 +21,7 @@ from tests.app.db import ( def test_last_template_usage_should_get_right_data(sample_notification): - results = dao_get_last_template_usage(sample_notification.template_id, 'sms') + results = dao_get_last_template_usage(sample_notification.template_id, 'sms', sample_notification.service_id) assert results.template.name == 'Template Name' assert results.template.template_type == 'sms' assert results.created_at == sample_notification.created_at @@ -36,12 +36,12 @@ def test_last_template_usage_should_be_able_to_get_all_template_usage_history_or ): template = create_template(sample_service, template_type=notification_type) - create_notification(template) - create_notification(template) - create_notification(template) + create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=1)) + create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=2)) + create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=3)) most_recent = create_notification(template) - results = dao_get_last_template_usage(template.id, notification_type) + results = dao_get_last_template_usage(template.id, notification_type, template.service_id) assert results.id == most_recent.id @@ -62,13 +62,13 @@ def test_last_template_usage_should_ignore_test_keys( created_at=one_minute_ago, api_key=sample_test_api_key) - results = dao_get_last_template_usage(sample_template.id, 'sms') + results = dao_get_last_template_usage(sample_template.id, 'sms', sample_template.service_id) assert results.id == team_key.id def test_last_template_usage_should_be_able_to_get_no_template_usage_history_if_no_notifications_using_template( sample_template): - results = dao_get_last_template_usage(sample_template.id, 'sms') + results = dao_get_last_template_usage(sample_template.id, 'sms', sample_template.service_id) assert not results