Merge pull request #2211 from alphagov/fix_notification_statistics_on_dashboard

Fix notification statistics on dashboard

app/dao/fact_notification_status_dao.py

@@ -4,11 +4,11 @@ from flask import current_app
 from sqlalchemy import func
 from sqlalchemy.dialects.postgresql import insert
 from sqlalchemy.sql.expression import literal
-from sqlalchemy.types import DateTime
+from sqlalchemy.types import DateTime, Integer
 
 from app import db
 from app.models import Notification, NotificationHistory, FactNotificationStatus, KEY_TYPE_TEST
-from app.utils import convert_bst_to_utc, get_london_midnight_in_utc
+from app.utils import convert_bst_to_utc, get_london_midnight_in_utc, midnight_n_days_ago
 
 
 def fetch_notification_status_for_day(process_day, service_id=None):
@@ -111,3 +111,39 @@ def fetch_notification_status_for_service_for_day(bst_day, service_id):
         Notification.notification_type,
         Notification.status
     ).all()
+
+
+def fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, limit_days=7):
+    start_date = midnight_n_days_ago(limit_days)
+    now = datetime.utcnow()
+    stats_for_7_days = db.session.query(
+        FactNotificationStatus.notification_type.label('notification_type'),
+        FactNotificationStatus.notification_status.label('status'),
+        FactNotificationStatus.notification_count.label('count')
+    ).filter(
+        FactNotificationStatus.service_id == service_id,
+        FactNotificationStatus.bst_date >= start_date,
+        FactNotificationStatus.key_type != KEY_TYPE_TEST
+    )
+
+    stats_for_today = db.session.query(
+        Notification.notification_type.cast(db.Text),
+        Notification.status,
+        func.count().label('count')
+    ).filter(
+        Notification.created_at >= get_london_midnight_in_utc(now),
+        Notification.service_id == service_id,
+        Notification.key_type != KEY_TYPE_TEST
+    ).group_by(
+        Notification.notification_type,
+        Notification.status
+    )
+    all_stats_table = stats_for_7_days.union_all(stats_for_today).subquery()
+    return db.session.query(
+        all_stats_table.c.notification_type,
+        all_stats_table.c.status,
+        func.cast(func.sum(all_stats_table.c.count), Integer).label('count'),
+    ).group_by(
+        all_stats_table.c.notification_type,
+        all_stats_table.c.status,
+    ).all()

app/service/rest.py

@@ -20,7 +20,8 @@ from app.dao.api_key_dao import (
     expire_api_key)
 from app.dao.fact_notification_status_dao import (
     fetch_notification_status_for_service_by_month,
-    fetch_notification_status_for_service_for_day
+    fetch_notification_status_for_service_for_day,
+    fetch_notification_status_for_service_for_today_and_7_previous_days
 )
 from app.dao.inbound_numbers_dao import dao_allocate_number_for_service
 from app.dao.organisation_dao import dao_get_organisation_by_service_id
@@ -47,7 +48,6 @@ from app.dao.services_dao import (
     dao_fetch_all_services_by_user,
     dao_fetch_monthly_historical_usage_by_template_for_service,
     dao_fetch_service_by_id,
-    dao_fetch_stats_for_service,
     dao_fetch_todays_stats_for_service,
     dao_fetch_todays_stats_for_all_services,
     dao_resume_service,
@@ -433,7 +433,7 @@ def get_service_statistics(service_id, today_only, limit_days=7):
     if today_only:
         stats = dao_fetch_todays_stats_for_service(service_id)
     else:
-        stats = dao_fetch_stats_for_service(service_id, limit_days=limit_days)
+        stats = fetch_notification_status_for_service_for_today_and_7_previous_days(service_id, limit_days=limit_days)
 
     return statistics.format_statistics(stats)
 

tests/app/dao/test_fact_notification_status_dao.py

@@ -6,8 +6,10 @@ from app.dao.fact_notification_status_dao import (
     fetch_notification_status_for_day,
     fetch_notification_status_for_service_by_month,
     fetch_notification_status_for_service_for_day,
+    fetch_notification_status_for_service_for_today_and_7_previous_days
 )
-from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM
+from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE
+from freezegun import freeze_time
 from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status
 
 
@@ -175,3 +177,46 @@ def test_fetch_notification_status_for_service_for_day(notify_db_session):
     assert results[1].notification_type == 'sms'
     assert results[1].notification_status == 'delivered'
     assert results[1].count == 1
+
+
+@freeze_time('2018-10-31T18:00:00')
+def test_fetch_notification_status_for_service_for_today_and_7_previous_days(notify_db_session):
+    service_1 = create_service(service_name='service_1')
+    sms_template = create_template(service=service_1, template_type=SMS_TYPE)
+    email_template = create_template(service=service_1, template_type=EMAIL_TYPE)
+
+    create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, count=10)
+    create_ft_notification_status(date(2018, 10, 24), 'sms', service_1, count=8)
+    create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, notification_status='created')
+    create_ft_notification_status(date(2018, 10, 29), 'email', service_1, count=3)
+    create_ft_notification_status(date(2018, 10, 26), 'letter', service_1, count=5)
+
+    create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))
+    create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status='delivered')
+    create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered')
+
+    # too early, shouldn't be included
+    create_notification(service_1.templates[0], created_at=datetime(2018, 10, 30, 12, 0, 0), status='delivered')
+
+    results = sorted(
+        fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id),
+        key=lambda x: (x.notification_type, x.status)
+    )
+
+    assert len(results) == 4
+
+    assert results[0].notification_type == 'email'
+    assert results[0].status == 'delivered'
+    assert results[0].count == 4
+
+    assert results[1].notification_type == 'letter'
+    assert results[1].status == 'delivered'
+    assert results[1].count == 5
+
+    assert results[2].notification_type == 'sms'
+    assert results[2].status == 'created'
+    assert results[2].count == 2
+
+    assert results[3].notification_type == 'sms'
+    assert results[3].status == 'delivered'
+    assert results[3].count == 19

tests/app/service/test_rest.py

@@ -34,6 +34,7 @@ from tests.app.conftest import (
     sample_notification_with_job
 )
 from tests.app.db import (
+    create_ft_notification_status,
     create_service,
     create_service_with_inbound_number,
     create_template,
@@ -1469,8 +1470,7 @@ def test_set_sms_prefixing_for_service_cant_be_none(
 ], ids=['seven_days', 'today'])
 def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_service, today_only, stats):
     with notify_api.test_request_context(), notify_api.test_client() as client:
-        with freeze_time('2000-01-01T12:00:00'):
-            create_sample_notification(notify_db, notify_db_session, status='delivered')
+        create_ft_notification_status(date(2000, 1, 1), 'sms', sample_service, count=1)
         with freeze_time('2000-01-02T12:00:00'):
             create_sample_notification(notify_db, notify_db_session, status='created')
             resp = client.get(

tests/app/service/test_statistics_rest.py

@@ -1,5 +1,5 @@
 import uuid
-from datetime import datetime
+from datetime import datetime, date
 
 import pytest
 from freezegun import freeze_time
@@ -133,9 +133,8 @@ def test_get_template_usage_by_month_returns_two_templates(admin_request, sample
     (False, {'requested': 2, 'delivered': 1, 'failed': 0}),
     (True, {'requested': 1, 'delivered': 0, 'failed': 0})
 ], ids=['seven_days', 'today'])
-def test_get_service_notification_statistics(admin_request, sample_template, today_only, stats):
-    with freeze_time('2000-01-01T12:00:00'):
-        create_notification(sample_template, status='delivered')
+def test_get_service_notification_statistics(admin_request, sample_service, sample_template, today_only, stats):
+    create_ft_notification_status(date(2000, 1, 1), 'sms', sample_service, count=1)
     with freeze_time('2000-01-02T12:00:00'):
         create_notification(sample_template, status='created')
         resp = admin_request.get(