From f5e38a896c665f0dd822f130647e934d5eb32d21 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 26 Jul 2017 12:09:13 +0100 Subject: [PATCH] Update the last template usage query to check Notification table: * Don't check the NotificationHistory table (this can cause a timeout) * Check template actually exists first --- app/dao/notifications_dao.py | 8 ++++---- app/template_statistics/rest.py | 19 +++++++++++++----- tests/app/template_statistics/test_rest.py | 23 +++++++++++++++++++--- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 231ceba59..c3333fc5a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -149,11 +149,11 @@ def dao_get_template_usage(service_id, limit_days=None): @statsd(namespace="dao") def dao_get_last_template_usage(template_id): - return NotificationHistory.query.filter( - NotificationHistory.template_id == template_id, - NotificationHistory.key_type != KEY_TYPE_TEST + return Notification.query.filter( + Notification.template_id == template_id, + Notification.key_type != KEY_TYPE_TEST ).order_by( - desc(NotificationHistory.created_at) + desc(Notification.created_at) ).first() diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index 16cd17672..78ae711e4 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -7,8 +7,12 @@ from flask import ( from app import redis_store from app.dao.notifications_dao import ( dao_get_template_usage, - dao_get_last_template_usage) -from app.dao.templates_dao import dao_get_templates_for_cache + dao_get_last_template_usage +) +from app.dao.templates_dao import ( + dao_get_templates_for_cache, + dao_get_template_by_id_and_service_id +) from app.schemas import notification_with_template_schema from app.utils import cache_key_for_service_template_counter @@ -52,12 +56,17 @@ def get_template_statistics_for_service_by_day(service_id): @template_statistics.route('/') def get_template_statistics_for_template_id(service_id, template_id): - notification = dao_get_last_template_usage(template_id) - if not notification: + template = dao_get_template_by_id_and_service_id(template_id, service_id) + if not template: message = 'No template found for id {}'.format(template_id) errors = {'template_id': [message]} raise InvalidRequest(errors, status_code=404) - data = notification_with_template_schema.dump(notification).data + + data = None + notification = dao_get_last_template_usage(template_id) + if notification: + data = notification_with_template_schema.dump(notification).data + return jsonify(data=data) diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index 4f79541ec..f1d2ef078 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -211,8 +211,8 @@ def test_get_template_statistics_by_id_returns_last_notification( def test_get_template_statistics_for_template_returns_empty_if_no_statistics( - client, - sample_template, + client, + sample_template, ): auth_header = create_authorization_header() @@ -221,7 +221,24 @@ def test_get_template_statistics_for_template_returns_empty_if_no_statistics( headers=[('Content-Type', 'application/json'), auth_header], ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['data'] == {} + + +def test_get_template_statistics_raises_error_for_nonexistent_template( + client, + sample_service, + fake_uuid +): + auth_header = create_authorization_header() + + response = client.get( + '/service/{}/template-statistics/{}'.format(sample_service.id, fake_uuid), + headers=[('Content-Type', 'application/json'), auth_header], + ) + assert response.status_code == 404 json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['message'] == 'No result found' assert json_resp['result'] == 'error' - assert json_resp['message']['template_id'] == ['No template found for id {}'.format(sample_template.id)]