Changed template stats for template id

- now returns the most recent notification history row for that template ID.
- contains all the required data for the use cases for that template

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)\
+        .join(Template) \
-        .group_by(table.template_id, Template.name, Template.template_type)\
+        .group_by(table.template_id, Template.name, Template.template_type) \
-        .order_by(asc(Template.name))\
+        .order_by(asc(Template.name)) \
         .all()
 
 
 @statsd(namespace="dao")
-def dao_get_template_statistics_for_service(service_id, limit_days=None):
+def dao_get_last_template_usage(template_id):
-    query_filter = [TemplateStatistics.service_id == service_id]
+    return NotificationHistory.query.filter(
-    if limit_days is not None:
+        NotificationHistory.template_id == template_id
-        query_filter.append(TemplateStatistics.day >= days_ago(limit_days))
+    ).join(Template) \
-    return TemplateStatistics.query.filter(*query_filter).order_by(
+        .order_by(desc(NotificationHistory.created_at)) \
-        desc(TemplateStatistics.updated_at)).all()
+        .first()
-
-
-@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()
 
 
 @statsd(namespace="dao")

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_last_template_usage)
-    dao_get_template_statistics_for_template
-)
 
-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('/<template_id>')
 def get_template_statistics_for_template_id(service_id, template_id):
-    stats = dao_get_template_statistics_for_template(template_id)
+    notification = dao_get_last_template_usage(template_id)
-    data = template_statistics_schema.dump(stats, many=True).data
+    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)

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

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
+        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(
+
+    notification_1 = sample_notification(
         notify_db,
         notify_db_session,
-        template_name='Sample Template 2',
+        service=sample_service,
-        service=sample_service
+        template=template)
-    )
+    notification_2 = sample_notification(
-
+        notify_db,
-    template_1_stats_1 = TemplateStatistics(
+        notify_db_session,
-        template_id=template_1.id,
+        service=sample_service,
-        service_id=sample_service.id,
+        template=template)
-        day=datetime(2001, 1, 1)
+    notification_3 = sample_notification(
-    )
+        notify_db,
-    template_1_stats_2 = TemplateStatistics(
+        notify_db_session,
-        template_id=template_1.id,
+        service=sample_service,
-        service_id=sample_service.id,
+        template=template)
-        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()
 
     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))
+            json_resp = json.loads(response.get_data(as_text=True))['data']
-            assert len(json_resp['data']) == 2
+            print(json_resp)
-            assert json_resp['data'][0]['id'] == str(template_1_stats_2.id)
+            assert json_resp['id'] == str(notification_3.id)
-            assert json_resp['data'][1]['id'] == str(template_1_stats_1.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)]