From 507138cc94ea0757342d82c50c2710f4714cc196 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 10 Jan 2019 16:24:51 +0000 Subject: [PATCH 1/3] Create a new query for template monthly stats. --- app/dao/fact_notification_status_dao.py | 84 ++++++++++++++++++- app/service/rest.py | 15 +++- .../dao/test_fact_notification_status_dao.py | 29 ++++++- tests/app/service/test_statistics_rest.py | 18 +--- 4 files changed, 123 insertions(+), 23 deletions(-) diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index bf6ec3c8e..7231dc7d8 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -4,12 +4,12 @@ from flask import current_app from notifications_utils.timezones import convert_bst_to_utc from sqlalchemy import func from sqlalchemy.dialects.postgresql import insert -from sqlalchemy.sql.expression import literal +from sqlalchemy.sql.expression import literal, extract from sqlalchemy.types import DateTime, Integer from app import db -from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST, Service -from app.utils import get_london_midnight_in_utc, midnight_n_days_ago +from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST, Service, Template +from app.utils import get_london_midnight_in_utc, midnight_n_days_ago, get_london_month_from_utc_column def fetch_notification_status_for_day(process_day, service_id=None): @@ -291,3 +291,81 @@ def fetch_stats_for_all_services_by_date_range(start_date, end_date, include_fro else: query = stats return query.all() + + +def fetch_monthly_template_usage_for_service(start_date, end_date, service_id): + # services_dao.replaces dao_fetch_monthly_historical_usage_by_template_for_service + stats = db.session.query( + FactNotificationStatus.template_id.label('template_id'), + Template.name.label('name'), + Template.template_type.label('template_type'), + Template.is_precompiled_letter.label('is_precompiled_letter'), + extract('month', FactNotificationStatus.bst_date).label('month'), + extract('year', FactNotificationStatus.bst_date).label('year'), + func.sum(FactNotificationStatus.notification_count).label('count') + ).join( + Template, FactNotificationStatus.template_id == Template.id + ).filter( + FactNotificationStatus.service_id == service_id, + FactNotificationStatus.bst_date >= start_date, + FactNotificationStatus.bst_date <= end_date, + ).group_by( + FactNotificationStatus.template_id, + Template.name, + Template.template_type, + Template.is_precompiled_letter, + extract('month', FactNotificationStatus.bst_date).label('month'), + extract('year', FactNotificationStatus.bst_date).label('year'), + ) + + if start_date <= datetime.utcnow() <= end_date: + today = get_london_midnight_in_utc(datetime.utcnow()) + month = get_london_month_from_utc_column(Notification.created_at) + + stats_for_today = db.session.query( + Notification.template_id.label('template_id'), + Template.name.label('name'), + Template.template_type.label('template_type'), + Template.is_precompiled_letter.label('is_precompiled_letter'), + extract('month', month).label('month'), + extract('year', month).label('year'), + func.count().label('count') + ).join( + Template, Notification.template_id == Template.id, + ).filter( + Notification.created_at >= today, + Notification.service_id == service_id, + # we don't want to include test keys + Notification.key_type != KEY_TYPE_TEST + ).group_by( + Notification.template_id, + Template.hidden, + Template.name, + Template.template_type, + month + ) + + all_stats_table = stats.union_all(stats_for_today).subquery() + query = db.session.query( + all_stats_table.c.template_id, + all_stats_table.c.name, + all_stats_table.c.is_precompiled_letter, + all_stats_table.c.template_type, + all_stats_table.c.month, + all_stats_table.c.year, + func.cast(func.sum(all_stats_table.c.count), Integer).label('count'), + ).group_by( + all_stats_table.c.template_id, + all_stats_table.c.name, + all_stats_table.c.is_precompiled_letter, + all_stats_table.c.template_type, + all_stats_table.c.month, + all_stats_table.c.year, + ).order_by( + all_stats_table.c.year, + all_stats_table.c.month, + all_stats_table.c.name + ) + else: + query = stats + return query.all() diff --git a/app/service/rest.py b/app/service/rest.py index 48fd6ae75..e00c99aa7 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -24,7 +24,8 @@ from app.dao.fact_notification_status_dao import ( fetch_notification_status_for_service_by_month, fetch_notification_status_for_service_for_day, fetch_notification_status_for_service_for_today_and_7_previous_days, - fetch_stats_for_all_services_by_date_range) + fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service +) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service from app.dao.organisation_dao import dao_get_organisation_by_service_id from app.dao.service_data_retention_dao import ( @@ -579,10 +580,16 @@ def resume_service(service_id): @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, - int(request.args.get('year', 'NaN')) + start_date, end_date = get_financial_year(int(request.args.get('year', 'NaN'))) + data = fetch_monthly_template_usage_for_service( + start_date=start_date, + end_date=end_date, + service_id=service_id ) + # data = dao_fetch_monthly_historical_usage_by_template_for_service( + # service_id, + # int(request.args.get('year', 'NaN')) + # ) stats = list() for i in data: diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 64b6b5bc3..0e541f7a9 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -11,7 +11,8 @@ from app.dao.fact_notification_status_dao import ( fetch_notification_status_for_service_for_today_and_7_previous_days, fetch_notification_status_totals_for_all_services, fetch_notification_statuses_for_job, - fetch_stats_for_all_services_by_date_range) + fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service +) from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE from freezegun import freeze_time from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status, create_job @@ -338,3 +339,29 @@ def test_fetch_stats_for_all_services_by_date_range(notify_db_session): assert not results[4].notification_type assert not results[4].status assert not results[4].count + + +def test_fetch_monthly_template_usage_for_service(sample_service): + template_one = create_template(service=sample_service, template_type='sms', template_name='one') + template_two = create_template(service=sample_service, template_type='email', template_name='one') + template_three = create_template(service=sample_service, template_type='letter', template_name='one') + + create_ft_notification_status(bst_date=date(2018, 1, 1), + service=sample_service, + template=template_one, + count=2) + create_ft_notification_status(bst_date=date(2018, 2, 1), + service=sample_service, + template=template_two, + count=3) + create_ft_notification_status(bst_date=date(2018, 3, 1), + service=sample_service, + template=template_three, + count=5) + + results = fetch_monthly_template_usage_for_service( + datetime(2017, 4, 1), datetime(2018, 3, 31), sample_service.id + ) + + print(results) + assert len(results) == 3 diff --git a/tests/app/service/test_statistics_rest.py b/tests/app/service/test_statistics_rest.py index 5b9719637..edf9e5b9a 100644 --- a/tests/app/service/test_statistics_rest.py +++ b/tests/app/service/test_statistics_rest.py @@ -28,13 +28,7 @@ def test_get_template_usage_by_month_returns_correct_data( admin_request, sample_template ): - create_notification(sample_template, created_at=datetime(2016, 4, 1), status='created') - create_notification(sample_template, created_at=datetime(2017, 4, 1), status='sending') - create_notification(sample_template, created_at=datetime(2017, 4, 1), status='permanent-failure') - create_notification(sample_template, created_at=datetime(2017, 4, 1), status='temporary-failure') - - daily_stats_template_usage_by_month() - + create_ft_notification_status(bst_date=date(2017, 4, 2), template=sample_template, count=3) create_notification(sample_template, created_at=datetime.utcnow()) resp_json = admin_request.get( @@ -85,14 +79,8 @@ def test_get_template_usage_by_month_returns_two_templates(admin_request, sample template_name=PRECOMPILED_TEMPLATE_NAME, hidden=True ) - - create_notification(template_one, created_at=datetime(2017, 4, 1), status='created') - create_notification(sample_template, created_at=datetime(2017, 4, 1), status='sending') - create_notification(sample_template, created_at=datetime(2017, 4, 1), status='permanent-failure') - create_notification(sample_template, created_at=datetime(2017, 4, 1), status='temporary-failure') - - daily_stats_template_usage_by_month() - + create_ft_notification_status(bst_date=datetime(2017, 4, 1), template=template_one, count=1) + create_ft_notification_status(bst_date=datetime(2017, 4, 1), template=sample_template, count=3) create_notification(sample_template, created_at=datetime.utcnow()) resp_json = admin_request.get( From c3c9d1eac987cd6b443641a7d0ec9fba148c005f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 11 Jan 2019 17:09:42 +0000 Subject: [PATCH 2/3] Add unit tests. Fix data types in result set. --- app/dao/fact_notification_status_dao.py | 4 +-- app/service/rest.py | 1 - .../dao/test_fact_notification_status_dao.py | 36 +++++++++++++++---- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 7231dc7d8..f5466bbec 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -351,8 +351,8 @@ def fetch_monthly_template_usage_for_service(start_date, end_date, service_id): all_stats_table.c.name, all_stats_table.c.is_precompiled_letter, all_stats_table.c.template_type, - all_stats_table.c.month, - all_stats_table.c.year, + func.cast(all_stats_table.c.month, Integer).label('month'), + func.cast(all_stats_table.c.year, Integer).label('year'), func.cast(func.sum(all_stats_table.c.count), Integer).label('count'), ).group_by( all_stats_table.c.template_id, diff --git a/app/service/rest.py b/app/service/rest.py index e00c99aa7..4aa0447b0 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -49,7 +49,6 @@ from app.dao.services_dao import ( dao_create_service, dao_fetch_all_services, dao_fetch_all_services_by_user, - dao_fetch_monthly_historical_usage_by_template_for_service, dao_fetch_service_by_id, dao_fetch_todays_stats_for_service, dao_fetch_todays_stats_for_all_services, diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 0e541f7a9..9b1f92f63 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -341,10 +341,11 @@ def test_fetch_stats_for_all_services_by_date_range(notify_db_session): assert not results[4].count +@freeze_time('2018-01-04 14:00') def test_fetch_monthly_template_usage_for_service(sample_service): - template_one = create_template(service=sample_service, template_type='sms', template_name='one') - template_two = create_template(service=sample_service, template_type='email', template_name='one') - template_three = create_template(service=sample_service, template_type='letter', template_name='one') + template_one = create_template(service=sample_service, template_type='sms', template_name='1_one') + template_two = create_template(service=sample_service, template_type='email', template_name='2_two') + template_three = create_template(service=sample_service, template_type='letter', template_name='3_three') create_ft_notification_status(bst_date=date(2018, 1, 1), service=sample_service, @@ -353,15 +354,38 @@ def test_fetch_monthly_template_usage_for_service(sample_service): create_ft_notification_status(bst_date=date(2018, 2, 1), service=sample_service, template=template_two, - count=3) + count=4) create_ft_notification_status(bst_date=date(2018, 3, 1), service=sample_service, template=template_three, count=5) - + create_notification(template=template_one) results = fetch_monthly_template_usage_for_service( datetime(2017, 4, 1), datetime(2018, 3, 31), sample_service.id ) - print(results) assert len(results) == 3 + + assert results[0].template_id == template_one.id + assert results[0].name == template_one.name + assert results[0].is_precompiled_letter is False + assert results[0].template_type == template_one.template_type + assert results[0].month == 1 + assert results[0].year == 2018 + assert results[0].count == 3 + + assert results[1].template_id == template_two.id + assert results[1].name == template_two.name + assert results[1].is_precompiled_letter is False + assert results[1].template_type == template_two.template_type + assert results[1].month == 2 + assert results[1].year == 2018 + assert results[1].count == 4 + + assert results[2].template_id == template_three.id + assert results[2].name == template_three.name + assert results[2].is_precompiled_letter is False + assert results[2].template_type == template_three.template_type + assert results[2].month == 3 + assert results[2].year == 2018 + assert results[2].count == 5 From b5a3ef9576e160c58c8715a738b397d7e0ffbb9f Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 14 Jan 2019 15:28:26 +0000 Subject: [PATCH 3/3] Added order by. Added more unit tests. Remove comments. --- app/dao/fact_notification_status_dao.py | 14 ++- app/service/rest.py | 5 - .../dao/test_fact_notification_status_dao.py | 116 ++++++++++++++---- 3 files changed, 105 insertions(+), 30 deletions(-) diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index f5466bbec..6bb341409 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -8,7 +8,10 @@ from sqlalchemy.sql.expression import literal, extract from sqlalchemy.types import DateTime, Integer from app import db -from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST, Service, Template +from app.models import ( + Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST, Service, Template, + NOTIFICATION_CANCELLED +) from app.utils import get_london_midnight_in_utc, midnight_n_days_ago, get_london_month_from_utc_column @@ -309,6 +312,7 @@ def fetch_monthly_template_usage_for_service(start_date, end_date, service_id): FactNotificationStatus.service_id == service_id, FactNotificationStatus.bst_date >= start_date, FactNotificationStatus.bst_date <= end_date, + FactNotificationStatus.notification_status != NOTIFICATION_CANCELLED ).group_by( FactNotificationStatus.template_id, Template.name, @@ -316,6 +320,10 @@ def fetch_monthly_template_usage_for_service(start_date, end_date, service_id): Template.is_precompiled_letter, extract('month', FactNotificationStatus.bst_date).label('month'), extract('year', FactNotificationStatus.bst_date).label('year'), + ).order_by( + extract('year', FactNotificationStatus.bst_date), + extract('month', FactNotificationStatus.bst_date), + Template.name ) if start_date <= datetime.utcnow() <= end_date: @@ -335,8 +343,8 @@ def fetch_monthly_template_usage_for_service(start_date, end_date, service_id): ).filter( Notification.created_at >= today, Notification.service_id == service_id, - # we don't want to include test keys - Notification.key_type != KEY_TYPE_TEST + Notification.key_type != KEY_TYPE_TEST, + Notification.status != NOTIFICATION_CANCELLED ).group_by( Notification.template_id, Template.hidden, diff --git a/app/service/rest.py b/app/service/rest.py index 4aa0447b0..279d00ed5 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -585,11 +585,6 @@ def get_monthly_template_usage(service_id): end_date=end_date, service_id=service_id ) - # data = dao_fetch_monthly_historical_usage_by_template_for_service( - # service_id, - # int(request.args.get('year', 'NaN')) - # ) - stats = list() for i in data: stats.append( diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 9b1f92f63..a7adffce0 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -341,51 +341,123 @@ def test_fetch_stats_for_all_services_by_date_range(notify_db_session): assert not results[4].count -@freeze_time('2018-01-04 14:00') +@freeze_time('2018-03-30 14:00') def test_fetch_monthly_template_usage_for_service(sample_service): - template_one = create_template(service=sample_service, template_type='sms', template_name='1_one') - template_two = create_template(service=sample_service, template_type='email', template_name='2_two') - template_three = create_template(service=sample_service, template_type='letter', template_name='3_three') + template_one = create_template(service=sample_service, template_type='sms', template_name='a') + template_two = create_template(service=sample_service, template_type='email', template_name='b') + template_three = create_template(service=sample_service, template_type='letter', template_name='c') + + create_ft_notification_status(bst_date=date(2017, 12, 10), + service=sample_service, + template=template_two, + count=3) + create_ft_notification_status(bst_date=date(2017, 12, 10), + service=sample_service, + template=template_one, + count=6) create_ft_notification_status(bst_date=date(2018, 1, 1), service=sample_service, template=template_one, - count=2) - create_ft_notification_status(bst_date=date(2018, 2, 1), - service=sample_service, - template=template_two, count=4) + create_ft_notification_status(bst_date=date(2018, 3, 1), service=sample_service, template=template_three, count=5) - create_notification(template=template_one) + create_notification(template=template_three, created_at=datetime.utcnow() - timedelta(days=1)) + create_notification(template=template_three, created_at=datetime.utcnow()) results = fetch_monthly_template_usage_for_service( datetime(2017, 4, 1), datetime(2018, 3, 31), sample_service.id ) - assert len(results) == 3 + assert len(results) == 4 assert results[0].template_id == template_one.id assert results[0].name == template_one.name assert results[0].is_precompiled_letter is False assert results[0].template_type == template_one.template_type - assert results[0].month == 1 - assert results[0].year == 2018 - assert results[0].count == 3 - + assert results[0].month == 12 + assert results[0].year == 2017 + assert results[0].count == 6 assert results[1].template_id == template_two.id assert results[1].name == template_two.name assert results[1].is_precompiled_letter is False assert results[1].template_type == template_two.template_type + assert results[1].month == 12 + assert results[1].year == 2017 + assert results[1].count == 3 + + assert results[2].template_id == template_one.id + assert results[2].name == template_one.name + assert results[2].is_precompiled_letter is False + assert results[2].template_type == template_one.template_type + assert results[2].month == 1 + assert results[2].year == 2018 + assert results[2].count == 4 + + assert results[3].template_id == template_three.id + assert results[3].name == template_three.name + assert results[3].is_precompiled_letter is False + assert results[3].template_type == template_three.template_type + assert results[3].month == 3 + assert results[3].year == 2018 + assert results[3].count == 6 + + +@freeze_time('2018-03-30 14:00') +def test_fetch_monthly_template_usage_for_service_does_join_to_notifications_if_today_is_not_in_date_range( + sample_service +): + template_one = create_template(service=sample_service, template_type='sms', template_name='a') + template_two = create_template(service=sample_service, template_type='email', template_name='b') + create_ft_notification_status(bst_date=date(2018, 2, 1), + service=template_two.service, + template=template_two, + count=15) + create_ft_notification_status(bst_date=date(2018, 2, 2), + service=template_one.service, + template=template_one, + count=20) + create_ft_notification_status(bst_date=date(2018, 3, 1), + service=template_one.service, + template=template_one, + count=3) + create_notification(template=template_one, created_at=datetime.utcnow()) + results = fetch_monthly_template_usage_for_service( + datetime(2018, 1, 1), datetime(2018, 2, 20), template_one.service_id + ) + + assert len(results) == 2 + + assert results[0].template_id == template_one.id + assert results[0].name == template_one.name + assert results[0].is_precompiled_letter == template_one.is_precompiled_letter + assert results[0].template_type == template_one.template_type + assert results[0].month == 2 + assert results[0].year == 2018 + assert results[0].count == 20 + assert results[1].template_id == template_two.id + assert results[1].name == template_two.name + assert results[1].is_precompiled_letter == template_two.is_precompiled_letter + assert results[1].template_type == template_two.template_type assert results[1].month == 2 assert results[1].year == 2018 - assert results[1].count == 4 + assert results[1].count == 15 - assert results[2].template_id == template_three.id - assert results[2].name == template_three.name - assert results[2].is_precompiled_letter is False - assert results[2].template_type == template_three.template_type - assert results[2].month == 3 - assert results[2].year == 2018 - assert results[2].count == 5 + +@freeze_time('2018-03-30 14:00') +def test_fetch_monthly_template_usage_for_service_does_not_include_cancelled_status( + sample_template +): + create_ft_notification_status(bst_date=date(2018, 3, 1), + service=sample_template.service, + template=sample_template, + notification_status='cancelled', + count=15) + create_notification(template=sample_template, created_at=datetime.utcnow(), status='cancelled') + results = fetch_monthly_template_usage_for_service( + datetime(2018, 1, 1), datetime(2018, 3, 31), sample_template.service_id + ) + + assert len(results) == 0