From c27edf5e3c5245ab0fc236a028641f72d0552411 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Tue, 14 Nov 2017 14:32:34 +0000 Subject: [PATCH] Added new endpoint to get the new template stats Added a new endpoint which combines the usage of the stats table and the data from the notifications tables, instead of using all the data from the notification_history table. This should speed up the query times and improve the page performance. - Updated to make the stats create and update function transactional as it actually wasn't committing the data to the table - Added the get from the stats table - Add a a method to combine the two results - Added the endpoint --- app/celery/scheduled_tasks.py | 2 +- app/config.py | 2 +- app/dao/services_dao.py | 94 ++++++++-- app/dao/stats_template_usage_by_month_dao.py | 18 +- app/models.py | 8 + app/service/rest.py | 35 ++-- tests/app/dao/test_services_dao.py | 177 +++++++++++++++++- .../test_stats_template_usage_by_month_dao.py | 36 +++- tests/app/service/test_rest.py | 133 +++++++++++++ 9 files changed, 466 insertions(+), 39 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 82dc30f76..697fd2eef 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -408,7 +408,7 @@ def check_job_status(): raise JobIncompleteError("Job(s) {} have not completed.".format(job_ids)) -@notify_celery.task(name='daily-stats-template_usage_by_month') +@notify_celery.task(name='daily-stats-template-usage-by-month') @statsd(namespace="tasks") def daily_stats_template_usage_by_month(): results = dao_fetch_monthly_historical_stats_by_template() diff --git a/app/config.py b/app/config.py index 16892ac28..86a635985 100644 --- a/app/config.py +++ b/app/config.py @@ -242,7 +242,7 @@ class Config(object): 'schedule': crontab(), 'options': {'queue': QueueNames.PERIODIC} }, - 'daily-stats-template_usage_by_month': { + 'daily-stats-template-usage-by-month': { 'task': 'daily-stats-template_usage_by_month', 'schedule': crontab(hour=0, minute=50), 'options': {'queue': QueueNames.PERIODIC} diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index d023fb297..b6df020d2 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -12,31 +12,33 @@ from app.dao.dao_utils import ( ) from app.dao.date_util import get_financial_year from app.dao.service_sms_sender_dao import insert_service_sms_sender +from app.dao.stats_template_usage_by_month_dao import dao_get_template_usage_stats_by_service from app.models import ( - ProviderStatistics, - VerifyCode, + AnnualBilling, ApiKey, + InboundNumber, + InvitedUser, + Job, + JobStatistics, + Notification, + NotificationHistory, + Permission, + ProviderStatistics, + Service, + ServicePermission, + ServiceSmsSender, + StatsTemplateUsageByMonth, Template, TemplateHistory, TemplateRedacted, - InboundNumber, - Job, - NotificationHistory, - Notification, - Permission, User, - InvitedUser, - Service, - ServicePermission, - KEY_TYPE_TEST, - NOTIFICATION_STATUS_TYPES, - TEMPLATE_TYPES, - JobStatistics, - SMS_TYPE, + VerifyCode, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, - ServiceSmsSender, - AnnualBilling + KEY_TYPE_TEST, + NOTIFICATION_STATUS_TYPES, + SMS_TYPE, + TEMPLATE_TYPES ) from app.service.statistics import format_monthly_template_notification_stats from app.statsd_decorators import statsd @@ -540,5 +542,61 @@ def dao_fetch_monthly_historical_stats_by_template(): month, year ).order_by( - NotificationHistory.template_id + year, + month ).all() + + +@transactional +@statsd(namespace="dao") +def dao_fetch_monthly_historical_usage_by_template_for_service(service_id): + + results = dao_get_template_usage_stats_by_service(service_id) + + stats = list() + for result in results: + stat = StatsTemplateUsageByMonth( + template_id=result.template_id, + month=result.month, + year=result.year, + count=result.count + ) + stats.append(stat) + + month = get_london_month_from_utc_column(Notification.created_at) + year = func.date_trunc("year", Notification.created_at) + start_date = datetime.combine(date.today(), time.min) + + today_results = db.session.query( + Notification.template_id, + extract('month', month).label('month'), + extract('year', year).label('year'), + func.count().label('count') + ).filter( + Notification.created_at >= start_date + ).group_by( + Notification.template_id, + month, + year + ).order_by( + Notification.template_id + ).all() + + for today_result in today_results: + add_to_stats = True + for stat in stats: + if today_result.template_id == stat.template_id: + stat.count = stat.count + today_result.count + add_to_stats = False + + if add_to_stats: + new_stat = StatsTemplateUsageByMonth( + template_id=today_result.template_id, + month=today_result.month, + year=today_result.year, + count=today_result.count + ) + + stats.append(new_stat) + + return stats diff --git a/app/dao/stats_template_usage_by_month_dao.py b/app/dao/stats_template_usage_by_month_dao.py index 2884e2c53..014fad218 100644 --- a/app/dao/stats_template_usage_by_month_dao.py +++ b/app/dao/stats_template_usage_by_month_dao.py @@ -1,7 +1,11 @@ from app import db -from app.models import StatsTemplateUsageByMonth +from app.statsd_decorators import statsd +from app.dao.dao_utils import transactional +from app.models import StatsTemplateUsageByMonth, Template +@transactional +@statsd(namespace="dao") def insert_or_update_stats_for_template(template_id, month, year, count): result = db.session.query( StatsTemplateUsageByMonth @@ -21,4 +25,16 @@ def insert_or_update_stats_for_template(template_id, month, year, count): year=year, count=count ) + db.session.add(monthly_stats) + + +@statsd(namespace="dao") +def dao_get_template_usage_stats_by_service(service_id): + return db.session.query( + StatsTemplateUsageByMonth + ).join( + Template, StatsTemplateUsageByMonth.template_id == Template.id + ).filter( + Template.service_id == service_id + ).all() diff --git a/app/models.py b/app/models.py index a62de1562..0af0c2fc3 100644 --- a/app/models.py +++ b/app/models.py @@ -1613,3 +1613,11 @@ class StatsTemplateUsageByMonth(db.Model): nullable=False, default=0 ) + + def serialize(self): + return { + 'template_id': str(self.template_id), + 'month': self.month, + 'year': self.year, + 'count': self.count + } diff --git a/app/service/rest.py b/app/service/rest.py index a17920d36..1da973ea9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -31,23 +31,25 @@ from app.dao.service_sms_sender_dao import ( dao_get_sms_senders_by_service_id, update_existing_sms_sender_with_inbound_number) from app.dao.services_dao import ( - dao_fetch_service_by_id, - dao_fetch_all_services, - dao_create_service, - dao_update_service, - dao_fetch_all_services_by_user, dao_add_user_to_service, - dao_remove_user_from_service, + dao_archive_service, + dao_create_service, + dao_fetch_all_services, + dao_fetch_all_services_by_user, + dao_fetch_monthly_historical_stats_for_service, + dao_fetch_monthly_historical_stats_by_template_for_service, + dao_fetch_monthly_historical_usage_by_template_for_service, + dao_fetch_service_by_id, dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, dao_fetch_todays_stats_for_all_services, - dao_archive_service, - fetch_stats_by_date_range_for_all_services, - dao_suspend_service, dao_resume_service, - dao_fetch_monthly_historical_stats_for_service, - dao_fetch_monthly_historical_stats_by_template_for_service, - fetch_aggregate_stats_by_date_range_for_all_services) + dao_remove_user_from_service, + dao_suspend_service, + dao_update_service, + fetch_aggregate_stats_by_date_range_for_all_services, + fetch_stats_by_date_range_for_all_services +) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, dao_add_and_commit_whitelisted_contacts, @@ -522,6 +524,15 @@ def resume_service(service_id): return '', 204 +@service_blueprint.route('//notifications/templates_usage/monthly', methods=['GET']) +def get_monthly_template_usage(service_id): + try: + data = dao_fetch_monthly_historical_usage_by_template_for_service(service_id) + return jsonify(stats=[i.serialize() for i in data]), 200 + except ValueError: + raise InvalidRequest('Year must be a number', status_code=400) + + @service_blueprint.route('//notifications/templates/monthly', methods=['GET']) def get_monthly_template_stats(service_id): service = dao_fetch_service_by_id(service_id) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 997301531..8cefe3829 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -7,6 +7,7 @@ from sqlalchemy.orm.exc import FlushError, NoResultFound from sqlalchemy.exc import IntegrityError from freezegun import freeze_time from app import db +from app.celery.scheduled_tasks import daily_stats_template_usage_by_month from app.dao.inbound_numbers_dao import ( dao_set_inbound_number_to_service, dao_get_available_inbound_numbers @@ -31,8 +32,8 @@ from app.dao.services_dao import ( dao_resume_service, dao_fetch_active_users_for_service, dao_fetch_service_by_inbound_number, - dao_fetch_monthly_historical_stats_by_template -) + dao_fetch_monthly_historical_stats_by_template, + dao_fetch_monthly_historical_usage_by_template_for_service) from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission from app.dao.users_dao import save_model_user from app.models import ( @@ -1036,3 +1037,175 @@ def test_dao_fetch_monthly_historical_stats_by_template(notify_db, notify_db_ses assert result[1].month == 10 assert result[1].year == 2017 assert result[1].count == 1 + + +def test_dao_fetch_monthly_historical_usage_by_template_for_service_no_stats_today(notify_db, notify_db_session, sample_service): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered' + ) + + template_one = create_sample_template(notify_db, notify_db_session, template_name='1') + template_two = create_sample_template(notify_db, notify_db_session, template_name='2') + + n = notification_history(created_at=datetime(2017, 10, 1), sample_template=template_one) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime.now(), sample_template=template_two) + + daily_stats_template_usage_by_month() + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id), + key=lambda x: (x.month, x.year) + ) + + assert len(result) == 2 + + assert result[0].template_id == template_two.id + assert result[0].month == 4 + assert result[0].year == 2016 + assert result[0].count == 2 + + assert result[1].template_id == template_one.id + assert result[1].month == 10 + assert result[1].year == 2017 + assert result[1].count == 1 + + +@freeze_time("2017-11-10 11:09:00.000000") +def test_dao_fetch_monthly_historical_usage_by_template_for_service_add_to_historical(notify_db, notify_db_session, sample_service): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered' + ) + + template_one = create_sample_template(notify_db, notify_db_session, template_name='1') + template_two = create_sample_template(notify_db, notify_db_session, template_name='2') + template_three = create_sample_template(notify_db, notify_db_session, template_name='3') + + date = datetime.now() + day = date.day + month = date.month + year = date.year + + n = notification_history(created_at=datetime(2017, 9, 1), sample_template=template_one) + notification_history(created_at=datetime(year, month, day) - timedelta(days=1), sample_template=template_two) + notification_history(created_at=datetime(year, month, day) - timedelta(days=1), sample_template=template_two) + + daily_stats_template_usage_by_month() + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id), + key=lambda x: (x.month, x.year) + ) + + assert len(result) == 2 + + assert result[0].template_id == template_one.id + assert result[0].month == 9 + assert result[0].year == 2017 + assert result[0].count == 1 + + assert result[1].template_id == template_two.id + assert result[1].month == 11 + assert result[1].year == 2017 + assert result[1].count == 2 + + create_notification( + notify_db, + notify_db_session, + service=sample_service, + template=template_three, + created_at=datetime.now() + ) + create_notification( + notify_db, + notify_db_session, + service=sample_service, + template=template_two, + created_at=datetime.now() + ) + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id), + key=lambda x: (x.month, x.year) + ) + + assert len(result) == 3 + + assert result[0].template_id == template_one.id + assert result[0].month == 9 + assert result[0].year == 2017 + assert result[0].count == 1 + + assert result[1].template_id == template_two.id + assert result[1].month == month + assert result[1].year == year + assert result[1].count == 3 + + assert result[2].template_id == template_three.id + assert result[2].month == 11 + assert result[2].year == 2017 + assert result[2].count == 1 + + +@freeze_time("2017-11-10 11:09:00.000000") +def test_dao_fetch_monthly_historical_usage_by_template_for_service_does_add_old_notification(notify_db, notify_db_session, sample_service): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered' + ) + + template_one = create_sample_template(notify_db, notify_db_session, template_name='1') + template_two = create_sample_template(notify_db, notify_db_session, template_name='2') + template_three = create_sample_template(notify_db, notify_db_session, template_name='3') + + date = datetime.now() + day = date.day + month = date.month + year = date.year + + n = notification_history(created_at=datetime(2017, 9, 1), sample_template=template_one) + notification_history(created_at=datetime(year, month, day) - timedelta(days=1), sample_template=template_two) + notification_history(created_at=datetime(year, month, day) - timedelta(days=1), sample_template=template_two) + + daily_stats_template_usage_by_month() + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id), + key=lambda x: (x.month, x.year) + ) + + assert len(result) == 2 + + assert result[0].template_id == template_one.id + assert result[0].month == 9 + assert result[0].year == 2017 + assert result[0].count == 1 + + assert result[1].template_id == template_two.id + assert result[1].month == 11 + assert result[1].year == 2017 + assert result[1].count == 2 + + create_notification( + notify_db, + notify_db_session, + service=sample_service, + template=template_three, + created_at=datetime.utcnow() - timedelta(days=2) + ) + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id), + key=lambda x: (x.month, x.year) + ) + + assert len(result) == 2 diff --git a/tests/app/dao/test_stats_template_usage_by_month_dao.py b/tests/app/dao/test_stats_template_usage_by_month_dao.py index ae46c682b..778fb2137 100644 --- a/tests/app/dao/test_stats_template_usage_by_month_dao.py +++ b/tests/app/dao/test_stats_template_usage_by_month_dao.py @@ -1,9 +1,11 @@ -import pytest - -from app.dao.stats_template_usage_by_month_dao import insert_or_update_stats_for_template +from app import db +from app.dao.stats_template_usage_by_month_dao import ( + insert_or_update_stats_for_template, + dao_get_template_usage_stats_by_service +) from app.models import StatsTemplateUsageByMonth -from tests.app.conftest import sample_notification, sample_email_template, sample_template, sample_job, sample_service +from tests.app.db import create_service, create_template def test_create_stats_for_template(notify_db_session, sample_template): @@ -43,3 +45,29 @@ def test_update_stats_for_template(notify_db_session, sample_template): assert stats_by_month[1].month == 2 assert stats_by_month[1].year == 2017 assert stats_by_month[1].count == 30 + + +def test_dao_get_template_usage_stats_by_service(sample_service): + + email_template = create_template(service=sample_service, template_type="email") + + stats1 = StatsTemplateUsageByMonth( + template_id=email_template.id, + month=1, + year=2017, + count=10 + ) + + stats2 = StatsTemplateUsageByMonth( + template_id=email_template.id, + month=2, + year=2017, + count=10 + ) + + db.session.add(stats1) + db.session.add(stats2) + + result = dao_get_template_usage_stats_by_service(sample_service.id) + + assert len(result) == 2 diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index bd365b739..a85171ba7 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -8,6 +8,7 @@ import pytest from flask import url_for, current_app from freezegun import freeze_time +from app.celery.scheduled_tasks import daily_stats_template_usage_by_month from app.dao.services_dao import dao_remove_user_from_service from app.dao.templates_dao import dao_redact_template from app.dao.users_dao import save_model_user @@ -1927,6 +1928,138 @@ def test_get_template_stats_by_month_returns_correct_data(notify_db, notify_db_s assert resp_json["2016-05"][str(sample_template.id)]["counts"]["permanent-failure"] == 1 +def test_get_template_usage_by_month_returns_correct_data( + notify_db, + notify_db_session, + client, + sample_template +): + + # add a historical notification for template + not1 = create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + status='sending' + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + status='permanent-failure' + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + status='temporary-failure' + ) + + daily_stats_template_usage_by_month() + + create_notification( + sample_template, + created_at=datetime.utcnow() + ) + + resp = client.get( + '/service/{}/notifications/templates_usage/monthly?year=2016'.format(not1.service_id), + headers=[create_authorization_header()] + ) + resp_json = json.loads(resp.get_data(as_text=True)).get('stats') + + assert resp.status_code == 200 + assert len(resp_json) == 1 + + assert resp_json[0]["template_id"] == str(sample_template.id) + assert resp_json[0]["month"] == 4 + assert resp_json[0]["year"] == 2016 + assert resp_json[0]["count"] == 5 + + +def test_get_template_usage_by_month_returns_two_templates( + notify_db, + notify_db_session, + client, + sample_template, + sample_service +): + + template_one = create_template(sample_service) + + # add a historical notification for template + not1 = create_notification_history( + notify_db, + notify_db_session, + template_one, + created_at=datetime(2016, 4, 1), + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + status='sending' + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + status='permanent-failure' + ) + + create_notification_history( + notify_db, + notify_db_session, + sample_template, + created_at=datetime(2016, 4, 1), + status='temporary-failure' + ) + + daily_stats_template_usage_by_month() + + create_notification( + sample_template, + created_at=datetime.utcnow() + ) + + resp = client.get( + '/service/{}/notifications/templates_usage/monthly?year=2016'.format(not1.service_id), + headers=[create_authorization_header()] + ) + resp_json = json.loads(resp.get_data(as_text=True)).get('stats') + + assert resp.status_code == 200 + assert len(resp_json) == 2 + + resp_json = sorted(resp_json, key=lambda k: k.get('count', 0)) + + assert resp_json[0]["template_id"] == str(template_one.id) + assert resp_json[0]["month"] == 4 + assert resp_json[0]["year"] == 2016 + assert resp_json[0]["count"] == 1 + + assert resp_json[1]["template_id"] == str(sample_template.id) + assert resp_json[1]["month"] == 4 + assert resp_json[1]["year"] == 2016 + assert resp_json[1]["count"] == 4 + + @pytest.mark.parametrize('query_string, expected_status, expected_json', [ ('?year=abcd', 400, {'message': 'Year must be a number', 'result': 'error'}), ])