Merge pull request #2667 from alphagov/template-and-notification-stats-use-one-api-call

Use single api call for service notification stats and template usage stats

app/main/views/dashboard.py

@@ -28,6 +28,7 @@ from app import (
 from app.main import main
 from app.statistics_utils import add_rate_to_job, get_formatted_percentage
 from app.utils import (
+    DELIVERED_STATUSES,
     FAILURE_STATUSES,
     REQUESTED_STATUSES,
     Spreadsheet,
@@ -266,9 +267,13 @@ def get_inbox_partials(service_id):
     )}
 
 
+def filter_out_cancelled_stats(template_statistics):
+    return [s for s in template_statistics if s["status"] != "cancelled"]
+
+
 def aggregate_template_usage(template_statistics, sort_key='count'):
+    template_statistics = filter_out_cancelled_stats(template_statistics)
     templates = []
-    template_statistics = [s for s in template_statistics if s.get("status") != "cancelled"]
     for k, v in groupby(sorted(template_statistics, key=lambda x: x['template_id']), key=lambda x: x['template_id']):
         template_stats = list(v)
 
@@ -283,10 +288,26 @@ def aggregate_template_usage(template_statistics, sort_key='count'):
     return sorted(templates, key=lambda x: x[sort_key], reverse=True)
 
 
+def aggregate_notifications_stats(template_statistics):
+    template_statistics = filter_out_cancelled_stats(template_statistics)
+    notifications = {
+        template_type: {
+            status: 0 for status in ('requested', 'delivered', 'failed')
+        } for template_type in ["sms", "email", "letter"]
+    }
+    for stat in template_statistics:
+        notifications[stat["template_type"]]["requested"] += stat["count"]
+        if stat["status"] in DELIVERED_STATUSES:
+            notifications[stat["template_type"]]["delivered"] += stat["count"]
+        elif stat["status"] in FAILURE_STATUSES:
+            notifications[stat["template_type"]]["failed"] += stat["count"]
+
+    return notifications
+
+
 def get_dashboard_partials(service_id):
-    template_statistics = aggregate_template_usage(
-        template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7)
-    )
+    all_statistics = template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7)
+    template_statistics = aggregate_template_usage(all_statistics)
 
     scheduled_jobs, immediate_jobs = [], []
     if job_api_client.has_jobs(service_id):
@@ -296,7 +317,7 @@ def get_dashboard_partials(service_id):
             for job in job_api_client.get_immediate_jobs(service_id)
         ]
 
-    stats = service_api_client.get_service_statistics(service_id, today_only=False)
+    stats = aggregate_notifications_stats(all_statistics)
     column_width, max_notifiction_count = get_column_properties(
         number_of_columns=(
             3 if current_service.has_permission('letter') else 2

tests/app/main/views/test_dashboard.py

@@ -10,7 +10,9 @@ from flask import url_for
 from freezegun import freeze_time
 
 from app.main.views.dashboard import (
+    aggregate_notifications_stats,
     aggregate_status_types,
+    aggregate_template_usage,
     format_monthly_stats_to_list,
     format_template_stats_to_list,
     get_dashboard_totals,
@@ -35,6 +37,15 @@ stub_template_stats = [
         'template_type': 'sms',
         'template_name': 'one',
         'template_id': 'id-1',
+        'status': 'created',
+        'count': 50,
+        'is_precompiled_letter': False
+    },
+    {
+        'template_type': 'email',
+        'template_name': 'two',
+        'template_id': 'id-2',
+        'status': 'created',
         'count': 100,
         'is_precompiled_letter': False
     },
@@ -42,23 +53,50 @@ stub_template_stats = [
         'template_type': 'email',
         'template_name': 'two',
         'template_id': 'id-2',
-        'count': 200,
+        'status': 'technical-failure',
+        'count': 100,
         'is_precompiled_letter': False
     },
     {
         'template_type': 'letter',
         'template_name': 'three',
         'template_id': 'id-3',
+        'status': 'delivered',
         'count': 300,
         'is_precompiled_letter': False
     },
+    {
+        'template_type': 'sms',
+        'template_name': 'one',
+        'template_id': 'id-1',
+        'status': 'delivered',
+        'count': 50,
+        'is_precompiled_letter': False
+    },
     {
         'template_type': 'letter',
         'template_name': 'four',
         'template_id': 'id-4',
+        'status': 'delivered',
         'count': 400,
         'is_precompiled_letter': True
-    }
+    },
+    {
+        'template_type': 'letter',
+        'template_name': 'four',
+        'template_id': 'id-4',
+        'status': 'cancelled',
+        'count': 5,
+        'is_precompiled_letter': True
+    },
+    {
+        'template_type': 'letter',
+        'template_name': 'thirty-three',
+        'template_id': 'id-33',
+        'status': 'cancelled',
+        'count': 5,
+        'is_precompiled_letter': False
+    },
 ]
 
 
@@ -1036,9 +1074,7 @@ def test_route_for_service_permissions(
 
 
 def test_aggregate_template_stats():
-    from app.main.views.dashboard import aggregate_template_usage
     expected = aggregate_template_usage(copy.deepcopy(stub_template_stats))
-
     assert len(expected) == 4
     assert expected[0]['template_name'] == 'four'
     assert expected[0]['count'] == 400
@@ -1058,6 +1094,15 @@ def test_aggregate_template_stats():
     assert expected[3]['template_type'] == 'sms'
 
 
+def test_aggregate_notifications_stats():
+    expected = aggregate_notifications_stats(copy.deepcopy(stub_template_stats))
+    assert expected == {
+        "sms": {"requested": 100, "delivered": 50, "failed": 0},
+        "letter": {"requested": 700, "delivered": 700, "failed": 0},
+        "email": {"requested": 200, "delivered": 0, "failed": 100}
+    }
+
+
 def test_service_dashboard_updates_gets_dashboard_totals(
     mocker,
     logged_in_client,

tests/conftest.py

@@ -2273,7 +2273,8 @@ def mock_get_template_statistics(mocker, service_one, fake_uuid):
         "template_name": template['name'],
         "template_type": template['template_type'],
         "template_id": template['id'],
-        "is_precompiled_letter": False
+        "is_precompiled_letter": False,
+        "status": "delivered"
     }
 
     def _get_stats(service_id, limit_days=None):