diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d944a3202..86d5b986c 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -10,7 +10,6 @@ from werkzeug.datastructures import MultiDict from sqlalchemy import (desc, func, Integer, or_, and_, asc) from sqlalchemy.orm import joinedload from sqlalchemy.sql.expression import cast -from notifications_utils.template import get_sms_fragment_count from app import db from app.dao import days_ago @@ -136,6 +135,32 @@ def dao_get_7_day_agg_notification_statistics_for_service(service_id, ) +@statsd(namespace="dao") +def dao_get_template_usage(service_id, limit_days=None): + + table = NotificationHistory + + if limit_days and limit_days <= 7: # can get this data from notifications table + table = Notification + + query = db.session.query( + func.count(table.template_id).label('count'), + table.template_id, + Template.name, + Template.template_type + ) + + query_filter = [table.service_id == service_id] + if limit_days is not None: + query_filter.append(table.created_at >= days_ago(limit_days)) + + return query.filter(*query_filter) \ + .join(Template)\ + .group_by(table.template_id, Template.name, Template.template_type)\ + .order_by(asc(Template.name))\ + .all() + + @statsd(namespace="dao") def dao_get_template_statistics_for_service(service_id, limit_days=None): query_filter = [TemplateStatistics.service_id == service_id] diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index d4990ba04..191645b96 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -5,6 +5,7 @@ from flask import ( ) from app.dao.notifications_dao import ( + dao_get_template_usage, dao_get_template_statistics_for_service, dao_get_template_statistics_for_template ) @@ -21,7 +22,7 @@ register_errors(template_statistics) @template_statistics.route('') -def get_template_statistics_for_service(service_id): +def get_template_statistics_for_service_by_day(service_id): if request.args.get('limit_days'): try: limit_days = int(request.args['limit_days']) @@ -31,9 +32,17 @@ def get_template_statistics_for_service(service_id): raise InvalidRequest(message, status_code=400) else: limit_days = None - stats = dao_get_template_statistics_for_service(service_id, limit_days=limit_days) - data = template_statistics_schema.dump(stats, many=True).data - return jsonify(data=data) + stats = dao_get_template_usage(service_id, limit_days=limit_days) + + def serialize(data): + return { + 'count': data.count, + 'template_id': str(data.template_id), + 'template_name': data.name, + 'template_type': data.template_type + } + + return jsonify(data=[serialize(row) for row in stats]) @template_statistics.route('/') diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index cbed327cb..7a0aa8692 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -35,15 +35,16 @@ from app.dao.notifications_dao import ( dao_get_template_statistics_for_service, get_notifications_for_service, dao_get_7_day_agg_notification_statistics_for_service, dao_get_potential_notification_statistics_for_day, dao_get_notification_statistics_for_day, - dao_get_template_statistics_for_template, get_notification_by_id) + dao_get_template_statistics_for_template, get_notification_by_id, dao_get_template_usage) from notifications_utils.template import get_sms_fragment_count -from tests.app.conftest import (sample_notification) +from tests.app.conftest import (sample_notification, sample_template, sample_email_template, sample_service) def test_should_have_decorated_notifications_dao_functions(): assert dao_get_notification_statistics_for_service.__wrapped__.__name__ == 'dao_get_notification_statistics_for_service' # noqa + assert dao_get_template_usage.__wrapped__.__name__ == 'dao_get_template_usage' # noqa assert dao_get_notification_statistics_for_service_and_day.__wrapped__.__name__ == 'dao_get_notification_statistics_for_service_and_day' # noqa assert dao_get_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_notification_statistics_for_day' # noqa assert dao_get_potential_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_potential_notification_statistics_for_day' # noqa @@ -63,6 +64,137 @@ def test_should_have_decorated_notifications_dao_functions(): assert delete_notifications_created_more_than_a_week_ago.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago' # noqa +def test_should_by_able_to_get_template_count_from_notifications_history(notify_db, notify_db_session, sample_service): + sms = sample_template(notify_db, notify_db_session) + email = sample_email_template(notify_db, notify_db_session) + sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, service=sample_service, template=email) + + results = dao_get_template_usage(sample_service.id) + assert results[0].name == 'Email Template Name' + assert results[0].template_type == 'email' + assert results[0].count == 2 + + assert results[1].name == 'Template Name' + assert results[1].template_type == 'sms' + assert results[1].count == 3 + + +def test_should_by_able_to_get_template_count_from_notifications_history_for_service( + notify_db, + notify_db_session): + service_1 = sample_service(notify_db, notify_db_session, service_name="test1", email_from="test1") + service_2 = sample_service(notify_db, notify_db_session, service_name="test2", email_from="test2") + service_3 = sample_service(notify_db, notify_db_session, service_name="test3", email_from="test3") + + sms = sample_template(notify_db, notify_db_session) + + sample_notification(notify_db, notify_db_session, service=service_1, template=sms) + sample_notification(notify_db, notify_db_session, service=service_1, template=sms) + sample_notification(notify_db, notify_db_session, service=service_2, template=sms) + + assert dao_get_template_usage(service_1.id)[0].count == 2 + assert dao_get_template_usage(service_2.id)[0].count == 1 + assert len(dao_get_template_usage(service_3.id)) == 0 + + +def test_should_by_able_to_get_zero_count_from_notifications_history_if_no_rows(sample_service): + results = dao_get_template_usage(sample_service.id) + assert len(results) == 0 + + +def test_should_by_able_to_get_zero_count_from_notifications_history_if_no_service(): + results = dao_get_template_usage(str(uuid.uuid4())) + assert len(results) == 0 + + +def test_should_by_able_to_get_template_count_from_notifications_history_across_days( + notify_db, + notify_db_session, + sample_service): + sms = sample_template(notify_db, notify_db_session) + email = sample_email_template(notify_db, notify_db_session) + + today = datetime.now() + yesterday = datetime.now() - timedelta(days=1) + one_month_ago = datetime.now() - timedelta(days=30) + + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + + results = dao_get_template_usage(sample_service.id) + + assert len(results) == 2 + + assert [(row.name, row.template_type, row.count) for row in results] == [ + ('Email Template Name', 'email', 5), + ('Template Name', 'sms', 5) + ] + + +def test_should_by_able_to_get_template_count_from_notifications_history_with_day_limit( + notify_db, + notify_db_session, + sample_service): + sms = sample_template(notify_db, notify_db_session) + + email = sample_email_template(notify_db, notify_db_session) + + today = datetime.now() + yesterday = datetime.now() - timedelta(days=1) + one_month_ago = datetime.now() - timedelta(days=30) + + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + + results_day_one = dao_get_template_usage(sample_service.id, limit_days=0) + assert len(results_day_one) == 2 + + results_day_two = dao_get_template_usage(sample_service.id, limit_days=1) + assert len(results_day_two) == 2 + + results_day_30 = dao_get_template_usage(sample_service.id, limit_days=31) + assert len(results_day_30) == 2 + + assert [(row.name, row.template_type, row.count) for row in results_day_one] == [ + ('Email Template Name', 'email', 2), + ('Template Name', 'sms', 1) + ] + + assert [(row.name, row.template_type, row.count) for row in results_day_two] == [ + ('Email Template Name', 'email', 5), + ('Template Name', 'sms', 2), + ] + + assert [(row.name, row.template_type, row.count) for row in results_day_30] == [ + ('Email Template Name', 'email', 5), + ('Template Name', 'sms', 5), + ] + + def test_should_by_able_to_update_status_by_reference(sample_email_template, ses_provider): data = _notification_json(sample_email_template, status='sending') diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index 4d7b3384a..1544874f7 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, timedelta import json from freezegun import freeze_time @@ -6,124 +6,18 @@ from freezegun import freeze_time from app import db from app.models import TemplateStatistics from tests import create_authorization_header -from tests.app.conftest import sample_template as create_sample_template +from tests.app.conftest import sample_template as create_sample_template, sample_template, sample_notification, \ + sample_email_template -@freeze_time('2016-04-09') -def test_get_template_statistics_for_service_for_last_week(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - +def test_get_all_template_statistics_with_bad_arg_returns_400(notify_api, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: auth_header = create_authorization_header() response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header], - query_string={'limit_days': 7} - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 8 - assert json_resp['data'][0]['day'] == '2016-04-09' - assert json_resp['data'][6]['day'] == '2016-04-03' - - -@freeze_time('2016-04-30') -def test_get_template_statistics_for_service_for_last_week_with_no_data(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - auth_header = create_authorization_header() - - # Date is frozen at 2016-04-30 and no data written since - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header], - query_string={'limit_days': 7} - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 0 - - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header], - query_string={'limit_days': 30} - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 9 - - -def test_get_all_template_statistics_for_service(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - auth_header = create_authorization_header() - - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header] - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 9 - assert json_resp['data'][0]['day'] == '2016-04-09' - assert json_resp['data'][8]['day'] == '2016-04-01' - - -def test_get_all_template_statistics_with_bad_limit_arg_returns_400(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - auth_header = create_authorization_header() - - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), + '/service/{}/template-statistics'.format(sample_service.id), headers=[('Content-Type', 'application/json'), auth_header], query_string={'limit_days': 'blurk'} ) @@ -134,6 +28,135 @@ def test_get_all_template_statistics_with_bad_limit_arg_returns_400(notify_api, assert json_resp['message'] == {'limit_days': ['blurk is not an integer']} +@freeze_time('2016-08-18') +def test_get_template_statistics_for_service(notify_db, notify_db_session, notify_api, sample_service): + sms = sample_template(notify_db, notify_db_session, service=sample_service) + email = sample_email_template(notify_db, notify_db_session, service=sample_service) + today = datetime.now() + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + + auth_header = create_authorization_header() + + response = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 2 + assert json_resp['data'][0]['template_id'] == str(email.id) + assert json_resp['data'][0]['template_name'] == email.name + assert json_resp['data'][0]['template_type'] == email.template_type + assert json_resp['data'][1]['count'] == 2 + assert json_resp['data'][1]['template_id'] == str(sms.id) + assert json_resp['data'][1]['template_name'] == sms.name + assert json_resp['data'][1]['template_type'] == sms.template_type + + +@freeze_time('2016-08-18') +def test_get_template_statistics_for_service_limited_by_day(notify_db, notify_db_session, notify_api, sample_service): + sms = sample_template(notify_db, notify_db_session, service=sample_service) + email = sample_email_template(notify_db, notify_db_session, service=sample_service) + today = datetime.now() + a_week_ago = datetime.now() - timedelta(days=7) + a_month_ago = datetime.now() - timedelta(days=30) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=a_week_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=a_week_ago, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=a_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=a_month_ago, service=sample_service, template=email) + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + + auth_header = create_authorization_header() + + response = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + query_string={'limit_days': 1} + ) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 1 + assert json_resp['data'][0]['template_id'] == str(email.id) + assert json_resp['data'][0]['template_name'] == email.name + assert json_resp['data'][0]['template_type'] == email.template_type + assert json_resp['data'][1]['count'] == 1 + assert json_resp['data'][1]['template_id'] == str(sms.id) + assert json_resp['data'][1]['template_name'] == sms.name + assert json_resp['data'][1]['template_type'] == sms.template_type + + response_for_a_week = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + query_string={'limit_days': 7} + ) + + assert response.status_code == 200 + json_resp = json.loads(response_for_a_week.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 2 + assert json_resp['data'][0]['template_name'] == 'Email Template Name' + assert json_resp['data'][1]['count'] == 2 + assert json_resp['data'][1]['template_name'] == 'Template Name' + + response_for_a_month = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + query_string={'limit_days': 30} + ) + + assert response_for_a_month.status_code == 200 + json_resp = json.loads(response_for_a_month.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 3 + assert json_resp['data'][0]['template_name'] == 'Email Template Name' + assert json_resp['data'][1]['count'] == 3 + assert json_resp['data'][1]['template_name'] == 'Template Name' + + response_for_all = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response_for_all.status_code == 200 + json_resp = json.loads(response_for_all.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 3 + assert json_resp['data'][0]['template_name'] == 'Email Template Name' + assert json_resp['data'][1]['count'] == 3 + assert json_resp['data'][1]['template_name'] == 'Template Name' + + +@freeze_time('2016-08-18') +def test_returns_empty_list_if_no_templates_used(notify_api, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + + auth_header = create_authorization_header() + + response = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 0 + + def test_get_template_statistics_for_template_only_returns_for_provided_template( notify_db, notify_db_session,