From 5ba58031b18b9cb801ce45bcd9f55505abaf3ad1 Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Thu, 16 Nov 2017 13:51:42 +0000
Subject: [PATCH] Fixed Bug in stats aggregation

The check for aggregation was too broad and hence was adding together
totals based on template_id and not the unqiue combination of
template id, month and year.

- Added test to test for the failure
- Added check and a test to for template_id, mon and year matches
- Celery process name did not match the task
---
 app/dao/services_dao.py            |  6 ++-
 tests/app/dao/test_services_dao.py | 62 ++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py
index 0fe8be7d0..396f2a762 100644
--- a/app/dao/services_dao.py
+++ b/app/dao/services_dao.py
@@ -571,7 +571,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year)
         extract('year', year).label('year'),
         func.count().label('count')
     ).join(
-        Template, Notification.template_id == Template.id
+        Template, Notification.template_id == Template.id,
     ).filter(
         Notification.created_at >= start_date
     ).group_by(
@@ -586,7 +586,9 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year)
     for today_result in today_results:
         add_to_stats = True
         for stat in stats:
-            if today_result.template_id == stat.template_id:
+            if today_result.template_id == stat.template_id and \
+                            today_result.month == stat.month and \
+                            today_result.year == stat.year:
                 stat.count = stat.count + today_result.count
                 add_to_stats = False
 
diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py
index f880ba4e3..0539ca1cb 100644
--- a/tests/app/dao/test_services_dao.py
+++ b/tests/app/dao/test_services_dao.py
@@ -1292,3 +1292,65 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_get_this_yea
     )
 
     assert len(result) == 2
+
+
+@freeze_time("2017-11-10 11:09:00.000000")
+def test_dao_fetch_monthly_historical_usage_by_template_for_service_combined_historical_current(
+        notify_db,
+        notify_db_session,
+        sample_service
+):
+    notification_history = functools.partial(
+        create_notification_history,
+        notify_db,
+        notify_db_session,
+        status='delivered'
+    )
+
+    template_one = create_sample_template(notify_db, notify_db_session, template_name='1')
+
+    date = datetime.now()
+    day = date.day
+    month = date.month
+    year = date.year
+
+    n = notification_history(created_at=datetime(year, month, day) - timedelta(days=30), sample_template=template_one)
+
+    daily_stats_template_usage_by_month()
+
+    result = sorted(
+        dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id, 2017),
+        key=lambda x: (x.month, x.year)
+    )
+
+    assert len(result) == 1
+
+    assert result[0].template_id == template_one.id
+    assert result[0].month == 10
+    assert result[0].year == 2017
+    assert result[0].count == 1
+
+    create_notification(
+        notify_db,
+        notify_db_session,
+        service=sample_service,
+        template=template_one,
+        created_at=datetime.utcnow()
+    )
+
+    result = sorted(
+        dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id, 2017),
+        key=lambda x: (x.month, x.year)
+    )
+
+    assert len(result) == 2
+
+    assert result[0].template_id == template_one.id
+    assert result[0].month == 10
+    assert result[0].year == 2017
+    assert result[0].count == 1
+
+    assert result[1].template_id == template_one.id
+    assert result[1].month == 11
+    assert result[1].year == 2017
+    assert result[1].count == 1