diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 86d5b986c..03b629b53 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -137,7 +137,6 @@ 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 @@ -155,28 +154,19 @@ def dao_get_template_usage(service_id, limit_days=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))\ + .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] - if limit_days is not None: - query_filter.append(TemplateStatistics.day >= days_ago(limit_days)) - return TemplateStatistics.query.filter(*query_filter).order_by( - desc(TemplateStatistics.updated_at)).all() - - -@statsd(namespace="dao") -def dao_get_template_statistics_for_template(template_id): - return TemplateStatistics.query.filter( - TemplateStatistics.template_id == template_id - ).order_by( - desc(TemplateStatistics.updated_at) - ).all() +def dao_get_last_template_usage(template_id): + return NotificationHistory.query.filter( + NotificationHistory.template_id == template_id + ).join(Template) \ + .order_by(desc(NotificationHistory.created_at)) \ + .first() @statsd(namespace="dao") diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index 191645b96..87335c439 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -6,11 +6,9 @@ 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 -) + dao_get_last_template_usage) -from app.schemas import template_statistics_schema +from app.schemas import notifications_filter_schema, NotificationWithTemplateSchema, notification_with_template_schema template_statistics = Blueprint('template-statistics', __name__, @@ -47,6 +45,10 @@ def get_template_statistics_for_service_by_day(service_id): @template_statistics.route('/') def get_template_statistics_for_template_id(service_id, template_id): - stats = dao_get_template_statistics_for_template(template_id) - data = template_statistics_schema.dump(stats, many=True).data + notification = dao_get_last_template_usage(template_id) + if not notification: + 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 return jsonify(data=data) diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 7a0aa8692..9f2c9a90c 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -5,7 +5,6 @@ from functools import partial import pytest from freezegun import freeze_time -from mock import ANY from sqlalchemy.exc import SQLAlchemyError, IntegrityError from app import db @@ -32,10 +31,9 @@ from app.dao.notifications_dao import ( update_notification_status_by_id, update_provider_stats, update_notification_status_by_reference, - 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_usage) + get_notification_by_id, dao_get_template_usage, dao_get_last_template_usage) from notifications_utils.template import get_sms_fragment_count @@ -44,13 +42,12 @@ from tests.app.conftest import (sample_notification, sample_template, sample_ema 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_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_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 assert dao_get_7_day_agg_notification_statistics_for_service.__wrapped__.__name__ == 'dao_get_7_day_agg_notification_statistics_for_service' # noqa - assert dao_get_template_statistics_for_service.__wrapped__.__name__ == 'dao_get_template_statistics_for_service' # noqa - assert dao_get_template_statistics_for_template.__wrapped__.__name__ == 'dao_get_template_statistics_for_template' # 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 @@ -64,6 +61,44 @@ 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_be_able_to_get_template_usage_history(notify_db, notify_db_session, sample_service): + with freeze_time('2000-01-01 12:00:00'): + sms = sample_template(notify_db, notify_db_session) + notification = sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + results = dao_get_last_template_usage(sms.id) + assert results.template.name == 'Template Name' + assert results.template.template_type == 'sms' + assert results.created_at == datetime(year=2000, month=1, day=1, hour=12, minute=0, second=0) + assert results.template_id == sms.id + assert results.id == notification.id + + +def test_should_be_able_to_get_all_template_usage_history_order_by_notification_created_at( + notify_db, + notify_db_session, + sample_service): + + sms = sample_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) + most_recent = sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + + results = dao_get_last_template_usage(sms.id) + assert results.id == most_recent.id + + +def test_should_be_able_to_get_no_template_usage_history_if_no_notifications_using_template( + notify_db, + notify_db_session): + + sms = sample_template(notify_db, notify_db_session) + + results = dao_get_last_template_usage(sms.id) + assert not results + + 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) @@ -1012,103 +1047,6 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_ _assert_notification_stats(sample_template.service.id, sms_requested=3) -@freeze_time("2016-03-30") -def test_get_template_stats_for_service_returns_stats_in_reverse_date_order(sample_template, sample_job): - template_stats = dao_get_template_statistics_for_service(sample_template.service.id) - assert len(template_stats) == 0 - data = _notification_json(sample_template, job_id=sample_job.id) - - notification = Notification(**data) - dao_create_notification(notification, sample_template.template_type) - - # move on one day - with freeze_time('2016-03-31'): - new_notification = Notification(**data) - dao_create_notification(new_notification, sample_template.template_type) - - # move on one more day - with freeze_time('2016-04-01'): - new_notification = Notification(**data) - dao_create_notification(new_notification, sample_template.template_type) - - template_stats = dao_get_template_statistics_for_service(sample_template.service_id) - assert len(template_stats) == 3 - assert template_stats[0].day == date(2016, 4, 1) - assert template_stats[1].day == date(2016, 3, 31) - assert template_stats[2].day == date(2016, 3, 30) - - -@freeze_time('2016-04-09') -def test_get_template_stats_for_service_returns_stats_can_limit_number_of_days_returned(sample_template): - template_stats = dao_get_template_statistics_for_service(sample_template.service.id) - assert len(template_stats) == 0 - - # 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() - - # Retrieve last week of stats - template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=7) - assert len(template_stats) == 8 - assert template_stats[0].day == date(2016, 4, 9) - # Final day of stats should be the same as today, eg Monday - assert template_stats[0].day.isoweekday() == template_stats[7].day.isoweekday() - assert template_stats[7].day == date(2016, 4, 2) - - -@freeze_time('2016-04-09') -def test_get_template_stats_for_service_returns_stats_returns_all_stats_if_no_limit(sample_template): - template_stats = dao_get_template_statistics_for_service(sample_template.service.id) - assert len(template_stats) == 0 - - # 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() - - template_stats = dao_get_template_statistics_for_service(sample_template.service_id) - assert len(template_stats) == 9 - assert template_stats[0].day == date(2016, 4, 9) - assert template_stats[8].day == date(2016, 4, 1) - - -@freeze_time('2016-04-30') -def test_get_template_stats_for_service_returns_no_result_if_no_usage_within_limit_days(sample_template): - template_stats = dao_get_template_statistics_for_service(sample_template.service.id) - assert len(template_stats) == 0 - - # make 9 stats records from 1st to 9th April - no data after 10th - 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() - - # Retrieve a week of stats - read date is 2016-04-30 - template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=7) - assert len(template_stats) == 0 - - # Retrieve a month of stats - read date is 2016-04-30 - template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=30) - assert len(template_stats) == 9 - - -def test_get_template_stats_for_service_with_limit_if_no_records_returns_empty_list(sample_template): - template_stats = dao_get_template_statistics_for_service(sample_template.service.id, limit_days=7) - assert len(template_stats) == 0 - - @freeze_time("2016-01-10") def test_should_limit_notifications_return_by_day_limit_plus_one(notify_db, notify_db_session, sample_service): assert len(Notification.query.all()) == 0 diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index 1544874f7..5a4206d7f 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -13,7 +13,6 @@ from tests.app.conftest import sample_template as create_sample_template, sample 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( @@ -40,7 +39,6 @@ def test_get_template_statistics_for_service(notify_db, notify_db_session, notif with notify_api.test_request_context(): with notify_api.test_client() as client: - auth_header = create_authorization_header() response = client.get( @@ -77,7 +75,6 @@ def test_get_template_statistics_for_service_limited_by_day(notify_db, notify_db with notify_api.test_request_context(): with notify_api.test_client() as client: - auth_header = create_authorization_header() response = client.get( @@ -144,7 +141,6 @@ def test_get_template_statistics_for_service_limited_by_day(notify_db, notify_db 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( @@ -157,62 +153,48 @@ def test_returns_empty_list_if_no_templates_used(notify_api, sample_service): assert len(json_resp['data']) == 0 -def test_get_template_statistics_for_template_only_returns_for_provided_template( +def test_get_template_statistics_by_id_returns_last_notification( notify_db, notify_db_session, notify_api, - sample_service -): - template_1 = create_sample_template( + sample_service): + + template = create_sample_template( notify_db, notify_db_session, template_name='Sample Template 1', service=sample_service ) - template_2 = create_sample_template( + + notification_1 = sample_notification( notify_db, notify_db_session, - template_name='Sample Template 2', - service=sample_service - ) - - template_1_stats_1 = TemplateStatistics( - template_id=template_1.id, - service_id=sample_service.id, - day=datetime(2001, 1, 1) - ) - template_1_stats_2 = TemplateStatistics( - template_id=template_1.id, - service_id=sample_service.id, - day=datetime(2001, 1, 2) - ) - template_2_stats = TemplateStatistics( - template_id=template_2.id, - service_id=sample_service.id, - day=datetime(2001, 1, 1) - ) - - # separate commit to ensure stats_1 has earlier updated_at time - db.session.add(template_1_stats_1) - db.session.commit() - - db.session.add_all([template_1_stats_2, template_2_stats]) - db.session.commit() + service=sample_service, + template=template) + notification_2 = sample_notification( + notify_db, + notify_db_session, + service=sample_service, + template=template) + notification_3 = sample_notification( + notify_db, + notify_db_session, + service=sample_service, + template=template) 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, template_1.id), + '/service/{}/template-statistics/{}'.format(sample_service.id, template.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]['id'] == str(template_1_stats_2.id) - assert json_resp['data'][1]['id'] == str(template_1_stats_1.id) + json_resp = json.loads(response.get_data(as_text=True))['data'] + print(json_resp) + assert json_resp['id'] == str(notification_3.id) def test_get_template_statistics_for_template_returns_empty_if_no_statistics( @@ -221,36 +203,23 @@ def test_get_template_statistics_for_template_returns_empty_if_no_statistics( notify_api, sample_service ): - template_1 = create_sample_template( + template = create_sample_template( notify_db, notify_db_session, template_name='Sample Template 1', service=sample_service ) - template_2 = create_sample_template( - notify_db, - notify_db_session, - template_name='Sample Template 2', - service=sample_service - ) - - template_1_stats = TemplateStatistics( - template_id=template_1.id, - service_id=sample_service.id, - day=datetime(2001, 1, 1) - ) - db.session.add(template_1_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_service.id, template_2.id), + '/service/{}/template-statistics/{}'.format(sample_service.id, template.id), headers=[('Content-Type', 'application/json'), auth_header], ) - assert response.status_code == 200 + assert response.status_code == 404 json_resp = json.loads(response.get_data(as_text=True)) - assert json_resp['data'] == [] + assert json_resp['result'] == 'error' + assert json_resp['message']['template_id'] == ['No template found for id {}'.format(template.id)]