Added order by.

Added more unit tests.
Remove comments.

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,

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(

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_one = create_template(service=sample_service, template_type='sms', template_name='a')
-    template_two = create_template(service=sample_service, template_type='email', template_name='2_two')
+    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='3_three')
+    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].month == 12
-    assert results[0].year == 2018
+    assert results[0].year == 2017
-    assert results[0].count == 3
+    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
+@freeze_time('2018-03-30 14:00')
-    assert results[2].is_precompiled_letter is False
+def test_fetch_monthly_template_usage_for_service_does_not_include_cancelled_status(
-    assert results[2].template_type == template_three.template_type
+        sample_template
-    assert results[2].month == 3
+):
-    assert results[2].year == 2018
+    create_ft_notification_status(bst_date=date(2018, 3, 1),
-    assert results[2].count == 5
+                                  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