From 56d9c29e915fffd9274fb73714df1e0e86a8caf1 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 14 Jun 2016 16:47:22 +0100
Subject: [PATCH] Highlight failing jobs on the dashboard
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

> When we have jobs that have over 3% failure rates we should highlight
> those so that peoples attention is drawn to deal with the failure.
>
> They would then go to the job view to see what the details are where
> they could filter by failure, but that's a different story...
>
> This is just about calculating and highlighting those that need their
> attention.

— https://www.pivotaltracker.com/story/show/121206123

This commit:

- calculates the failure rate for each job
- makes jobs with a failure rate of > 3% go red on the dashboard
---
 app/main/views/dashboard.py              |  4 ++--
 app/statistics_utils.py                  | 18 ++++++++++++++++++
 app/templates/views/dashboard/_jobs.html |  4 ++--
 app/templates/views/dashboard/jobs.html  |  2 +-
 tests/app/test_statistics_utils.py       | 24 +++++++++++++++++++++++-
 5 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py
index 2ed337c21..ea4130e42 100644
--- a/app/main/views/dashboard.py
+++ b/app/main/views/dashboard.py
@@ -19,7 +19,7 @@ from app import (
     service_api_client,
     template_statistics_client
 )
-from app.statistics_utils import sum_of_statistics, add_rates_to
+from app.statistics_utils import sum_of_statistics, add_rates_to, add_rate_to_jobs
 from app.utils import user_has_permissions
 
 
@@ -169,5 +169,5 @@ def get_dashboard_statistics_for_service(service_id):
         'sms_allowance_remaining': max(0, (sms_free_allowance - sms_sent)),
         'sms_chargeable': max(0, sms_sent - sms_free_allowance),
         'sms_rate': sms_rate,
-        'jobs': job_api_client.get_job(service_id, limit_days=7)['data']
+        'jobs': add_rate_to_jobs(job_api_client.get_job(service_id, limit_days=7)['data'])
     }
diff --git a/app/statistics_utils.py b/app/statistics_utils.py
index eca91fb83..efe6ffe00 100644
--- a/app/statistics_utils.py
+++ b/app/statistics_utils.py
@@ -69,3 +69,21 @@ def statistics_by_state(statistics):
             'failed': statistics['emails_failed']
         }
     }
+
+
+def get_failure_rate_for_job(job):
+    if not job.get('notifications_delivered'):
+        if job.get('notifications_failed'):
+            return 1
+        return 0
+    return (
+        job.get('notifications_failed', 0) /
+        (job.get('notifications_failed', 0) + job.get('notifications_delivered', 0))
+    )
+
+
+def add_rate_to_jobs(jobs):
+    return [dict(
+        **job,
+        failure_rate=(get_failure_rate_for_job(job)) * 100
+    ) for job in jobs]
diff --git a/app/templates/views/dashboard/_jobs.html b/app/templates/views/dashboard/_jobs.html
index a197fda07..79b523a7b 100644
--- a/app/templates/views/dashboard/_jobs.html
+++ b/app/templates/views/dashboard/_jobs.html
@@ -19,7 +19,7 @@
       <div class="file-list">
         <a class="file-list-filename" href="{{ url_for('.view_job', service_id=current_service.id, job_id=item.id) }}">{{ item.original_file_name }}</a>
         <span class="file-list-hint">Uploaded {{ item.created_at|format_datetime_short }}</span>
-      </div
+      </div>
     {% endcall %}
     {% call field() %}
       {{ big_number(
@@ -30,7 +30,7 @@
     {% call field() %}
       {{ big_number(item.get('notifications_delivered', 0), smallest=True) }}
     {% endcall %}
-    {% call field(status='error' if 0 else '') %}
+    {% call field(status='error' if item.get('failure_rate', 0) > 3 else '') %}
       {{ big_number(item.get('notifications_failed', 0), smallest=True) }}
     {% endcall %}
   {% endcall %}
diff --git a/app/templates/views/dashboard/jobs.html b/app/templates/views/dashboard/jobs.html
index e32c15b3d..f06ce67aa 100644
--- a/app/templates/views/dashboard/jobs.html
+++ b/app/templates/views/dashboard/jobs.html
@@ -11,7 +11,7 @@
     {{ item.created_at|format_datetime }}
   {% endcall %}
   {% call field() %}
-    
+
   {% endcall %}
   {% call field(align='right') %}
     {{ item.notification_count }}
diff --git a/tests/app/test_statistics_utils.py b/tests/app/test_statistics_utils.py
index 839fbd8b1..1ea0d6542 100644
--- a/tests/app/test_statistics_utils.py
+++ b/tests/app/test_statistics_utils.py
@@ -1,6 +1,6 @@
 import pytest
 
-from app.statistics_utils import sum_of_statistics, add_rates_to, statistics_by_state
+from app.statistics_utils import sum_of_statistics, add_rates_to, add_rate_to_jobs, statistics_by_state
 
 
 @pytest.mark.parametrize('delivery_statistics', [
@@ -113,3 +113,25 @@ def test_service_statistics_by_state():
         assert resp[message_type]['sending'] == 1
         assert resp[message_type]['delivered'] == 1
         assert resp[message_type]['failed'] == 1
+
+
+@pytest.mark.parametrize('failed, delivered, expected_failure_rate', [
+    (0, 0, 0),
+    (0, 1, 0),
+    (1, 0, 100),
+    (1, 4, 20)
+])
+def test_add_rate_to_jobs(failed, delivered, expected_failure_rate):
+    resp = add_rate_to_jobs([
+        {
+            'notifications_failed': failed,
+            'notifications_delivered': delivered
+        },
+        {
+            'notifications_failed': 1,
+            'notifications_delivered': 1
+        }
+    ])
+
+    assert resp[0]['failure_rate'] == expected_failure_rate
+    assert resp[1]['failure_rate'] == 50