From db5663e429dc1ac3d6caa798181a1b1eec1e3527 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Mon, 2 Jul 2018 17:17:38 +0100
Subject: [PATCH] Improve the performance of the query to get the last template
 usage.

By adding the service id, the query performance has improved greatly. It went from 6200ms to 0.04ms.
This should stop the 500s when a template is deleted.
---
 app/dao/notifications_dao.py                       |  8 ++++++--
 app/template_statistics/rest.py                    |  2 +-
 .../test_notification_dao_template_usage.py        | 14 +++++++-------
 3 files changed, 14 insertions(+), 10 deletions(-)

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