Merge pull request #2249 from alphagov/improve-performance-of-platform-admin-page

Improve performance of platform stats page

app/dao/fact_notification_status_dao.py

@@ -138,3 +138,51 @@ def fetch_notification_status_for_service_for_today_and_7_previous_days(service_
         all_stats_table.c.notification_type,
         all_stats_table.c.status,
     ).all()
+
+
+def fetch_notification_status_totals_for_all_services(start_date, end_date):
+    stats = db.session.query(
+        FactNotificationStatus.notification_type.label('notification_type'),
+        FactNotificationStatus.notification_status.label('status'),
+        FactNotificationStatus.key_type.label('key_type'),
+        func.sum(FactNotificationStatus.notification_count).label('count')
+    ).filter(
+        FactNotificationStatus.bst_date >= start_date,
+        FactNotificationStatus.bst_date <= end_date
+    ).group_by(
+        FactNotificationStatus.notification_type,
+        FactNotificationStatus.notification_status,
+        FactNotificationStatus.key_type,
+    ).order_by(
+        FactNotificationStatus.notification_type
+    )
+    today = get_london_midnight_in_utc(datetime.utcnow())
+    if start_date <= today.date() <= end_date:
+        stats_for_today = db.session.query(
+            Notification.notification_type.cast(db.Text).label('notification_type'),
+            Notification.status,
+            Notification.key_type,
+            func.count().label('count')
+        ).filter(
+            Notification.created_at >= today
+        ).group_by(
+            Notification.notification_type.cast(db.Text),
+            Notification.status,
+            Notification.key_type,
+        )
+        all_stats_table = stats.union_all(stats_for_today).subquery()
+        query = db.session.query(
+            all_stats_table.c.notification_type,
+            all_stats_table.c.status,
+            all_stats_table.c.key_type,
+            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_stats_table.c.key_type,
+        ).order_by(
+            all_stats_table.c.notification_type
+        )
+    else:
+        query = stats
+    return query.all()

app/dao/notifications_dao.py

@@ -7,26 +7,25 @@ from datetime import (
 
 from boto.exception import BotoClientError
 from flask import current_app
-
+from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES
 from notifications_utils.recipients import (
     validate_and_format_email_address,
     InvalidEmailError,
     try_validate_and_format_phone_number
 )
 from notifications_utils.statsd_decorators import statsd
-from werkzeug.datastructures import MultiDict
+from notifications_utils.timezones import convert_utc_to_bst
 from sqlalchemy import (desc, func, or_, asc)
 from sqlalchemy.orm import joinedload
-from sqlalchemy.sql.expression import case
 from sqlalchemy.sql import functions
-from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES
-from notifications_utils.timezones import convert_utc_to_bst
+from sqlalchemy.sql.expression import case
+from werkzeug.datastructures import MultiDict
 
 from app import db, create_uuid
 from app.aws.s3 import remove_s3_object, get_s3_bucket_objects
-from app.letters.utils import LETTERS_PDF_FILE_LOCATION_STRUCTURE
-from app.utils import midnight_n_days_ago, escape_special_characters
+from app.dao.dao_utils import transactional
 from app.errors import InvalidRequest
+from app.letters.utils import LETTERS_PDF_FILE_LOCATION_STRUCTURE
 from app.models import (
     Notification,
     NotificationHistory,
@@ -47,9 +46,8 @@ from app.models import (
     EMAIL_TYPE,
     ServiceDataRetention
 )
-
-from app.dao.dao_utils import transactional
 from app.utils import get_london_midnight_in_utc
+from app.utils import midnight_n_days_ago, escape_special_characters
 
 
 @statsd(namespace="dao")
@@ -645,31 +643,3 @@ def guess_notification_type(search_term):
         return EMAIL_TYPE
     else:
         return SMS_TYPE
-
-
-@statsd(namespace='dao')
-def fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date):
-    start_date = get_london_midnight_in_utc(start_date)
-    end_date = get_london_midnight_in_utc(end_date + timedelta(days=1))
-    table = NotificationHistory
-
-    if start_date >= datetime.utcnow() - timedelta(days=7):
-        table = Notification
-
-    query = db.session.query(
-        table.notification_type,
-        table.status,
-        table.key_type,
-        func.count(table.id).label('count')
-    ).filter(
-        table.created_at >= start_date,
-        table.created_at < end_date
-    ).group_by(
-        table.notification_type,
-        table.key_type,
-        table.status
-    ).order_by(
-        table.notification_type,
-    )
-
-    return query.all()

app/platform_stats/rest.py

@@ -2,7 +2,7 @@ from datetime import datetime
 
 from flask import Blueprint, jsonify, request
 
-from app.dao.notifications_dao import fetch_aggregate_stats_by_date_range_for_all_services
+from app.dao.fact_notification_status_dao import fetch_notification_status_totals_for_all_services
 from app.errors import register_errors
 from app.platform_stats.platform_stats_schema import platform_stats_request
 from app.service.statistics import format_admin_stats
@@ -23,7 +23,7 @@ def get_platform_stats():
 
     start_date = datetime.strptime(request.args.get('start_date', today), '%Y-%m-%d').date()
     end_date = datetime.strptime(request.args.get('end_date', today), '%Y-%m-%d').date()
-    data = fetch_aggregate_stats_by_date_range_for_all_services(start_date=start_date, end_date=end_date)
+    data = fetch_notification_status_totals_for_all_services(start_date=start_date, end_date=end_date)
     stats = format_admin_stats(data)
 
     return jsonify(stats)

tests/app/dao/notification_dao/test_notification_dao.py

@@ -1,5 +1,5 @@
 import uuid
-from datetime import datetime, timedelta, date
+from datetime import datetime, timedelta
 from functools import partial
 
 import pytest
@@ -35,7 +35,6 @@ from app.dao.notifications_dao import (
     dao_get_notifications_by_references,
     dao_get_notification_history_by_reference,
     notifications_not_yet_sent,
-    fetch_aggregate_stats_by_date_range_for_all_services,
 )
 from app.dao.services_dao import dao_update_service
 from app.models import (
@@ -1787,51 +1786,3 @@ def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_
 
     results = notifications_not_yet_sent(older_than, notification_type)
     assert len(results) == 0
-
-
-def test_fetch_aggregate_stats_by_date_range_for_all_services_returns_empty_list_when_no_stats(notify_db_session):
-    start_date = date(2018, 1, 1)
-    end_date = date(2018, 1, 5)
-
-    result = fetch_aggregate_stats_by_date_range_for_all_services(start_date, end_date)
-    assert result == []
-
-
-@freeze_time('2018-01-08')
-def test_fetch_aggregate_stats_by_date_range_for_all_services_groups_stats(
-    sample_template,
-    sample_email_template,
-    sample_letter_template,
-):
-    today = datetime.now().date()
-
-    for i in range(3):
-        create_notification(template=sample_email_template, status='permanent-failure',
-                            created_at=today)
-
-    create_notification(template=sample_email_template, status='sent', created_at=today)
-    create_notification(template=sample_template, status='sent', created_at=today)
-    create_notification(template=sample_template, status='sent', created_at=today,
-                        key_type=KEY_TYPE_TEAM)
-    create_notification(template=sample_letter_template, status='virus-scan-failed',
-                        created_at=today)
-
-    result = fetch_aggregate_stats_by_date_range_for_all_services(today, today)
-
-    assert len(result) == 5
-    assert ('email', 'permanent-failure', 'normal', 3) in result
-    assert ('email', 'sent', 'normal', 1) in result
-    assert ('sms', 'sent', 'normal', 1) in result
-    assert ('sms', 'sent', 'team', 1) in result
-    assert ('letter', 'virus-scan-failed', 'normal', 1) in result
-
-
-def test_fetch_aggregate_stats_by_date_range_for_all_services_uses_bst_date(sample_template):
-    query_day = datetime(2018, 6, 5).date()
-    create_notification(sample_template, status='sent', created_at=datetime(2018, 6, 4, 23, 59))
-    create_notification(sample_template, status='created', created_at=datetime(2018, 6, 5, 23, 00))
-
-    result = fetch_aggregate_stats_by_date_range_for_all_services(query_day, query_day)
-
-    assert len(result) == 1
-    assert result[0].status == 'sent'

tests/app/dao/test_fact_notification_status_dao.py

@@ -1,14 +1,17 @@
 from datetime import timedelta, datetime, date
 from uuid import UUID
 
+import pytest
+
 from app.dao.fact_notification_status_dao import (
     update_fact_notification_status,
     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
+    fetch_notification_status_for_service_for_today_and_7_previous_days,
+    fetch_notification_status_totals_for_all_services
 )
-from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE
+from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
 from freezegun import freeze_time
 from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status
 
@@ -220,3 +223,68 @@ def test_fetch_notification_status_for_service_for_today_and_7_previous_days(not
     assert results[3].notification_type == 'sms'
     assert results[3].status == 'delivered'
     assert results[3].count == 19
+
+
+@pytest.mark.parametrize(
+    "start_date, end_date, expected_email, expected_letters, expected_sms, expected_created_sms",
+    [
+        (29, 30, 3, 10, 10, 1),  # not including today
+        (29, 31, 4, 10, 11, 2),  # today included
+        (26, 31, 4, 15, 11, 2),
+    ]
+
+)
+@freeze_time('2018-10-31 14:00')
+def test_fetch_notification_status_totals_for_all_services(
+        notify_db_session,
+        start_date,
+        end_date,
+        expected_email,
+        expected_letters,
+        expected_sms,
+        expected_created_sms
+):
+    set_up_data()
+
+    results = sorted(
+        fetch_notification_status_totals_for_all_services(
+            start_date=date(2018, 10, start_date), end_date=date(2018, 10, end_date)),
+        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 == expected_email
+
+    assert results[1].notification_type == 'letter'
+    assert results[1].status == 'delivered'
+    assert results[1].count == expected_letters
+
+    assert results[2].notification_type == 'sms'
+    assert results[2].status == 'created'
+    assert results[2].count == expected_created_sms
+
+    assert results[3].notification_type == 'sms'
+    assert results[3].status == 'delivered'
+    assert results[3].count == expected_sms
+
+
+def set_up_data():
+    service_2 = create_service(service_name='service_2')
+    create_template(service=service_2, template_type=LETTER_TYPE)
+    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, 24), 'sms', service_1, count=8)
+    create_ft_notification_status(date(2018, 10, 26), 'letter', service_1, count=5)
+    create_ft_notification_status(date(2018, 10, 29), 'sms', service_1, count=10)
+    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, 29), 'letter', service_2, count=10)
+
+    create_notification(service_1.templates[0], created_at=datetime(2018, 10, 30, 12, 0, 0), status='delivered')
+    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')

tests/app/platform_stats/test_rest.py

@@ -2,11 +2,14 @@ from datetime import date, datetime
 
 from freezegun import freeze_time
 
+from app.models import SMS_TYPE, EMAIL_TYPE
+from tests.app.db import create_service, create_template, create_ft_notification_status, create_notification
+
 
 @freeze_time('2018-06-01')
 def test_get_platform_stats_uses_todays_date_if_no_start_or_end_date_is_provided(admin_request, mocker):
     today = datetime.now().date()
-    dao_mock = mocker.patch('app.platform_stats.rest.fetch_aggregate_stats_by_date_range_for_all_services')
+    dao_mock = mocker.patch('app.platform_stats.rest.fetch_notification_status_totals_for_all_services')
     mocker.patch('app.service.rest.statistics.format_statistics')
 
     admin_request.get('platform_stats.get_platform_stats')
@@ -17,7 +20,7 @@ def test_get_platform_stats_uses_todays_date_if_no_start_or_end_date_is_provided
 def test_get_platform_stats_can_filter_by_date(admin_request, mocker):
     start_date = date(2017, 1, 1)
     end_date = date(2018, 1, 1)
-    dao_mock = mocker.patch('app.platform_stats.rest.fetch_aggregate_stats_by_date_range_for_all_services')
+    dao_mock = mocker.patch('app.platform_stats.rest.fetch_notification_status_totals_for_all_services')
     mocker.patch('app.service.rest.statistics.format_statistics')
 
     admin_request.get('platform_stats.get_platform_stats', start_date=start_date, end_date=end_date)
@@ -35,3 +38,37 @@ def test_get_platform_stats_validates_the_date(admin_request):
 
     assert response['errors'][0]['message'] == 'start_date time data {} does not match format %Y-%m-%d'.format(
         start_date)
+
+
+@freeze_time('2018-10-31 14:00')
+def test_get_platform_stats_with_real_query(admin_request, 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, 29), 'email', service_1, count=3)
+
+    create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0), key_type='test')
+    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')
+
+    response = admin_request.get(
+        'platform_stats.get_platform_stats', start_date=date(2018, 10, 29),
+    )
+    assert response == {
+        'email': {
+            'failures': {
+                'virus-scan-failed': 0, 'temporary-failure': 0, 'permanent-failure': 0, 'technical-failure': 0},
+            'total': 4, 'test-key': 0
+        },
+        'letter': {
+            'failures': {
+                'virus-scan-failed': 0, 'temporary-failure': 0, 'permanent-failure': 0, 'technical-failure': 0},
+            'total': 0, 'test-key': 0
+        },
+        'sms': {
+            'failures': {
+                'virus-scan-failed': 0, 'temporary-failure': 0, 'permanent-failure': 0, 'technical-failure': 0},
+            'total': 11, 'test-key': 1
+        }
+    }