From 3ce0024eeccc773444b2fabe5b9748c4e85415e3 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 15 Jan 2019 12:15:20 +0000 Subject: [PATCH] Remove unused functions for getting template statistics --- app/dao/notifications_dao.py | 34 ----- app/dao/templates_dao.py | 15 -- app/template_statistics/rest.py | 68 +-------- .../notification_dao/test_notification_dao.py | 2 - .../test_notification_dao_template_usage.py | 136 +----------------- tests/app/dao/test_templates_dao.py | 16 --- 6 files changed, 6 insertions(+), 265 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 9426a28c0..15c9e997a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -30,7 +30,6 @@ from app.models import ( Notification, NotificationHistory, ScheduledNotification, - Template, KEY_TYPE_TEST, LETTER_TYPE, NOTIFICATION_CREATED, @@ -51,39 +50,6 @@ from app.utils import get_london_midnight_in_utc from app.utils import midnight_n_days_ago, escape_special_characters -@statsd(namespace="dao") -def dao_get_template_usage(service_id, day): - start = get_london_midnight_in_utc(day) - end = get_london_midnight_in_utc(day + timedelta(days=1)) - - notifications_aggregate_query = db.session.query( - func.count().label('count'), - Notification.template_id - ).filter( - Notification.created_at >= start, - Notification.created_at < end, - Notification.service_id == service_id, - Notification.key_type != KEY_TYPE_TEST, - ).group_by( - Notification.template_id - ).subquery() - - query = db.session.query( - Template.id, - Template.name, - Template.template_type, - Template.is_precompiled_letter, - func.coalesce(notifications_aggregate_query.c.count, 0).label('count') - ).outerjoin( - notifications_aggregate_query, - notifications_aggregate_query.c.template_id == Template.id - ).filter( - Template.service_id == service_id - ).order_by(Template.name) - - return query.all() - - @statsd(namespace="dao") 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. diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index e5e93199f..66cbe865a 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -129,18 +129,3 @@ def dao_get_template_versions(service_id, template_id): ).order_by( desc(TemplateHistory.version) ).all() - - -def dao_get_multiple_template_details(template_ids): - query = db.session.query( - Template.id, - Template.template_type, - Template.name, - Template.is_precompiled_letter - ).filter( - Template.id.in_(template_ids) - ).order_by( - Template.name - ) - - return query.all() diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index f1c2ff1f0..fc179b49a 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -1,25 +1,10 @@ -from flask import ( - Blueprint, - jsonify, - request, - current_app -) - -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_multiple_template_details, - dao_get_template_by_id_and_service_id -) +from flask import Blueprint, jsonify, request +from app.dao.notifications_dao import dao_get_last_template_usage +from app.dao.templates_dao import dao_get_template_by_id_and_service_id from app.dao.fact_notification_status_dao import fetch_notification_status_for_service_for_today_and_7_previous_days from app.schemas import notification_with_template_schema -from app.utils import cache_key_for_service_template_usage_per_day, last_n_days from app.errors import register_errors, InvalidRequest -from collections import Counter template_statistics = Blueprint('template_statistics', __name__, @@ -67,50 +52,3 @@ def get_template_statistics_for_template_id(service_id, template_id): data = notification_with_template_schema.dump(notification).data return jsonify(data=data) - - -def _get_template_statistics_for_last_n_days(service_id, whole_days): - template_stats_by_id = Counter() - - # 0 whole_days = last 1 days (ie since midnight today) = today. - # 7 whole days = last 8 days (ie since midnight this day last week) = a week and a bit - for day in last_n_days(whole_days + 1): - # "{SERVICE_ID}-template-usage-{YYYY-MM-DD}" - key = cache_key_for_service_template_usage_per_day(service_id, day) - stats = redis_store.get_all_from_hash(key) - if stats: - stats = { - k.decode('utf-8'): int(v) for k, v in stats.items() - } - else: - # key didn't exist (or redis was down) - lets populate from DB. - stats = { - str(row.id): row.count for row in dao_get_template_usage(service_id, day=day) - } - # if there is data in db, but not in redis - lets put it in redis so we don't have to do - # this calc again next time. If there isn't any data, we can't put it in redis. - # Zero length hashes aren't a thing in redis. (There'll only be no data if the service has no templates) - # Nothing is stored if redis is down. - if stats: - redis_store.set_hash_and_expire( - key, - stats, - current_app.config['EXPIRE_CACHE_EIGHT_DAYS'] - ) - template_stats_by_id += Counter(stats) - - # attach count from stats to name/type/etc from database - template_details = dao_get_multiple_template_details(template_stats_by_id.keys()) - return [ - { - 'count': template_stats_by_id[str(template.id)], - 'template_id': str(template.id), - 'template_name': template.name, - 'template_type': template.template_type, - 'is_precompiled_letter': template.is_precompiled_letter - } - for template in template_details - # we don't want to return templates with no count to the front-end, - # but they're returned from the DB and might be put in redis like that (if there was no data that day) - if template_stats_by_id[str(template.id)] != 0 - ] diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 197b6556a..90dc73035 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -16,7 +16,6 @@ from app.dao.notifications_dao import ( dao_get_last_template_usage, dao_get_notifications_by_to_field, dao_get_scheduled_notifications, - dao_get_template_usage, dao_timeout_notifications, dao_update_notification, dao_update_notifications_by_reference, @@ -70,7 +69,6 @@ from tests.app.db import ( def test_should_have_decorated_notifications_dao_functions(): assert dao_get_last_template_usage.__wrapped__.__name__ == 'dao_get_last_template_usage' # noqa - assert dao_get_template_usage.__wrapped__.__name__ == 'dao_get_template_usage' # noqa assert dao_create_notification.__wrapped__.__name__ == 'dao_create_notification' # noqa assert update_notification_status_by_id.__wrapped__.__name__ == 'update_notification_status_by_id' # noqa assert dao_update_notification.__wrapped__.__name__ == 'dao_update_notification' # noqa 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 88aaa783d..4006fd9c2 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 @@ -1,23 +1,7 @@ -import uuid -from datetime import datetime, timedelta, date - +from datetime import datetime, timedelta import pytest -from freezegun import freeze_time - -from app.dao.notifications_dao import ( - dao_get_last_template_usage, - dao_get_template_usage -) -from app.models import ( - KEY_TYPE_NORMAL, - KEY_TYPE_TEST, - KEY_TYPE_TEAM -) -from tests.app.db import ( - create_notification, - create_service, - create_template -) +from app.dao.notifications_dao import dao_get_last_template_usage +from tests.app.db import create_notification, create_template def test_last_template_usage_should_get_right_data(sample_notification): @@ -70,117 +54,3 @@ def test_last_template_usage_should_be_able_to_get_no_template_usage_history_if_ sample_template): results = dao_get_last_template_usage(sample_template.id, 'sms', sample_template.service_id) assert not results - - -@freeze_time('2018-01-01') -def test_should_by_able_to_get_template_count(sample_template, sample_email_template): - create_notification(sample_template) - create_notification(sample_template) - create_notification(sample_template) - create_notification(sample_email_template) - create_notification(sample_email_template) - - results = dao_get_template_usage(sample_template.service_id, date.today()) - assert results[0].name == sample_email_template.name - assert results[0].template_type == sample_email_template.template_type - assert results[0].count == 2 - - assert results[1].name == sample_template.name - assert results[1].template_type == sample_template.template_type - assert results[1].count == 3 - - -@freeze_time('2018-01-01') -def test_template_usage_should_ignore_test_keys( - sample_team_api_key, - sample_test_api_key, - sample_api_key, - sample_template -): - - create_notification(sample_template, api_key=sample_api_key, key_type=KEY_TYPE_NORMAL) - create_notification(sample_template, api_key=sample_team_api_key, key_type=KEY_TYPE_TEAM) - create_notification(sample_template, api_key=sample_test_api_key, key_type=KEY_TYPE_TEST) - create_notification(sample_template) - - results = dao_get_template_usage(sample_template.service_id, date.today()) - assert results[0].name == sample_template.name - assert results[0].template_type == sample_template.template_type - assert results[0].count == 3 - - -def test_template_usage_should_filter_by_service(notify_db_session): - service_1 = create_service(service_name='test1') - service_2 = create_service(service_name='test2') - service_3 = create_service(service_name='test3') - - template_1 = create_template(service_1) - template_2 = create_template(service_2) # noqa - template_3a = create_template(service_3, template_name='a') - template_3b = create_template(service_3, template_name='b') # noqa - - # two for service_1, one for service_3 - create_notification(template_1) - create_notification(template_1) - - create_notification(template_3a) - - res1 = dao_get_template_usage(service_1.id, date.today()) - res2 = dao_get_template_usage(service_2.id, date.today()) - res3 = dao_get_template_usage(service_3.id, date.today()) - - assert len(res1) == 1 - assert res1[0].count == 2 - - assert len(res2) == 1 - assert res2[0].count == 0 - - assert len(res3) == 2 - assert res3[0].count == 1 - assert res3[1].count == 0 - - -def test_template_usage_should_by_able_to_get_zero_count_from_notifications_history_if_no_rows(sample_service): - results = dao_get_template_usage(sample_service.id, date.today()) - assert len(results) == 0 - - -def test_template_usage_should_by_able_to_get_zero_count_from_notifications_history_if_no_service(): - results = dao_get_template_usage(str(uuid.uuid4()), date.today()) - assert len(results) == 0 - - -def test_template_usage_should_by_able_to_get_template_count_for_specific_day(sample_template): - # too early - create_notification(sample_template, created_at=datetime(2017, 6, 7, 22, 59, 0)) - # just right - create_notification(sample_template, created_at=datetime(2017, 6, 7, 23, 0, 0)) - create_notification(sample_template, created_at=datetime(2017, 6, 7, 23, 0, 0)) - create_notification(sample_template, created_at=datetime(2017, 6, 8, 22, 59, 0)) - create_notification(sample_template, created_at=datetime(2017, 6, 8, 22, 59, 0)) - create_notification(sample_template, created_at=datetime(2017, 6, 8, 22, 59, 0)) - # too late - create_notification(sample_template, created_at=datetime(2017, 6, 8, 23, 0, 0)) - - results = dao_get_template_usage(sample_template.service_id, day=date(2017, 6, 8)) - - assert len(results) == 1 - assert results[0].count == 5 - - -def test_template_usage_should_by_able_to_get_template_count_for_specific_timezone_boundary(sample_template): - # too early - create_notification(sample_template, created_at=datetime(2018, 3, 24, 23, 59, 0)) - # just right - create_notification(sample_template, created_at=datetime(2018, 3, 25, 0, 0, 0)) - create_notification(sample_template, created_at=datetime(2018, 3, 25, 0, 0, 0)) - create_notification(sample_template, created_at=datetime(2018, 3, 25, 22, 59, 0)) - create_notification(sample_template, created_at=datetime(2018, 3, 25, 22, 59, 0)) - create_notification(sample_template, created_at=datetime(2018, 3, 25, 22, 59, 0)) - # too late - create_notification(sample_template, created_at=datetime(2018, 3, 25, 23, 0, 0)) - - results = dao_get_template_usage(sample_template.service_id, day=date(2018, 3, 25)) - - assert len(results) == 1 - assert results[0].count == 5 diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index cbe6c6b72..f585e2b5d 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -11,7 +11,6 @@ from app.dao.templates_dao import ( dao_get_all_templates_for_service, dao_update_template, dao_get_template_versions, - dao_get_multiple_template_details, dao_redact_template, dao_update_template_reply_to ) from app.models import ( @@ -511,21 +510,6 @@ def test_get_template_versions_is_empty_for_hidden_templates(notify_db, notify_d assert len(versions) == 0 -def test_get_multiple_template_details_returns_templates_for_list_of_ids(sample_service): - t1 = create_template(sample_service) - t2 = create_template(sample_service) - create_template(sample_service) # t3 - - res = dao_get_multiple_template_details([t1.id, t2.id]) - - assert {x.id for x in res} == {t1.id, t2.id} - # make sure correct properties are on each row - assert res[0].id - assert res[0].template_type - assert res[0].name - assert not res[0].is_precompiled_letter - - @pytest.mark.parametrize("template_type,postage", [('letter', 'third'), ('sms', 'second')]) def test_template_postage_constraint_on_create(sample_service, sample_user, template_type, postage): data = {