Merge pull request #268 from alphagov/align-statistics

Align date range for template + notification stats

app/dao/notifications_dao.py

@@ -39,10 +39,15 @@ def get_sms_message_count(char_count):
     return 1 if char_count <= 160 else math.ceil(float(char_count) / 153)
 
 
-def dao_get_notification_statistics_for_service(service_id):
+def dao_get_notification_statistics_for_service(service_id, limit_days=None):
-    return NotificationStatistics.query.filter_by(
+    filter = [NotificationStatistics.service_id == service_id]
-        service_id=service_id
+    if limit_days is not None:
-    ).order_by(desc(NotificationStatistics.day)).all()
+        filter.append(NotificationStatistics.day >= days_ago(limit_days))
+    return NotificationStatistics.query.filter(
+        *filter
+    ).order_by(
+        desc(NotificationStatistics.day)
+    ).all()
 
 
 def dao_get_notification_statistics_for_service_and_day(service_id, day):
@@ -52,24 +57,10 @@ def dao_get_notification_statistics_for_service_and_day(service_id, day):
     ).order_by(desc(NotificationStatistics.day)).first()
 
 
-def dao_get_notification_statistics_for_service_and_previous_days(service_id, limit_days):
-    return NotificationStatistics.query.filter_by(
-        service_id=service_id
-    ).filter(
-        NotificationStatistics.day.in_((
-            (date.today() - timedelta(days=days_ago))
-            for days_ago in range(0, limit_days + 1)
-        ))
-    ).order_by(
-        desc(NotificationStatistics.day)
-    ).all()
-
-
 def dao_get_template_statistics_for_service(service_id, limit_days=None):
     filter = [TemplateStatistics.service_id == service_id]
-    if limit_days:
+    if limit_days is not None:
-        last_date_to_fetch = date.today() - timedelta(days=limit_days)
+        filter.append(TemplateStatistics.day >= days_ago(limit_days))
-        filter.append(TemplateStatistics.day > last_date_to_fetch)
     return TemplateStatistics.query.filter(*filter).order_by(
         desc(TemplateStatistics.updated_at)).all()
 
@@ -264,3 +255,7 @@ def delete_notifications_created_more_than_a_week_ago(status):
     ).delete(synchronize_session='fetch')
     db.session.commit()
     return deleted
+
+
+def days_ago(number_of_days):
+    return date.today() - timedelta(days=number_of_days)

app/notifications_statistics/rest.py

@@ -4,10 +4,7 @@ from flask import (
     request
 )
 
-from app.dao.notifications_dao import (
+from app.dao.notifications_dao import dao_get_notification_statistics_for_service
-    dao_get_notification_statistics_for_service,
-    dao_get_notification_statistics_for_service_and_previous_days
-)
 from app.schemas import notifications_statistics_schema
 
 notifications_statistics = Blueprint(
@@ -25,7 +22,7 @@ def get_all_notification_statistics_for_service(service_id):
 
     if request.args.get('limit_days'):
         try:
-            statistics = dao_get_notification_statistics_for_service_and_previous_days(
+            statistics = dao_get_notification_statistics_for_service(
                 service_id=service_id,
                 limit_days=int(request.args['limit_days'])
             )

tests/app/dao/test_notification_dao.py

@@ -24,7 +24,6 @@ from app.dao.notifications_dao import (
     dao_get_notification_statistics_for_service,
     delete_notifications_created_more_than_a_week_ago,
     dao_get_notification_statistics_for_service_and_day,
-    dao_get_notification_statistics_for_service_and_previous_days,
     update_notification_status_by_id,
     update_notification_reference_by_id,
     update_notification_status_by_reference,
@@ -316,7 +315,7 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_pre
     dao_create_notification(notification_2, sample_template.template_type, mmg_provider_name)
     dao_create_notification(notification_3, sample_template.template_type, mmg_provider_name)
 
-    stats = dao_get_notification_statistics_for_service_and_previous_days(
+    stats = dao_get_notification_statistics_for_service(
         sample_template.service.id, 7
     )
     assert len(stats) == 2
@@ -1000,7 +999,7 @@ def test_get_template_stats_for_service_returns_stats_can_limit_number_of_days_r
     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
+    # 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):
@@ -1011,9 +1010,11 @@ def test_get_template_stats_for_service_returns_stats_can_limit_number_of_days_r
 
     # Retrieve last week of stats
     template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=7)
-    assert len(template_stats) == 7
+    assert len(template_stats) == 8
     assert template_stats[0].day == date(2016, 4, 9)
-    assert template_stats[6].day == date(2016, 4, 3)
+    # 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')

tests/app/template_statistics/test_rest.py

@@ -35,7 +35,7 @@ def test_get_template_statistics_for_service_for_last_week(notify_api, sample_te
 
             assert response.status_code == 200
             json_resp = json.loads(response.get_data(as_text=True))
-            assert len(json_resp['data']) == 7
+            assert len(json_resp['data']) == 8
             assert json_resp['data'][0]['day'] == '2016-04-09'
             assert json_resp['data'][6]['day'] == '2016-04-03'