Merge pull request #815 from alphagov/weekly-refactor

Weekly stats aggregate page refactor

app/main/views/dashboard.py

@@ -1,7 +1,8 @@
-from datetime import datetime, date, timedelta
+from datetime import datetime, timedelta
 from collections import namedtuple
 from itertools import groupby
 
+import dateutil
 from flask import (
     render_template,
     url_for,
@@ -9,13 +10,11 @@ from flask import (
     jsonify,
     current_app
 )
-
 from flask_login import login_required
 
 from app.main import main
 from app import (
     job_api_client,
-    statistics_api_client,
     service_api_client,
     template_statistics_client
 )
@@ -90,20 +89,10 @@ def usage(service_id):
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
 def weekly(service_id):
-
-    earliest_date = date(2016, 4, 1)  # start of tax year
-    while earliest_date.weekday() != 0:  # 0 for monday
-        earliest_date -= timedelta(days=1)
-
+    stats = service_api_client.get_weekly_notification_stats(service_id)['data']
     return render_template(
         'views/weekly.html',
-        days=(
-            add_rates_to(day) for day in
-            statistics_api_client.get_7_day_aggregate_for_service(
-                service_id,
-                date_from=earliest_date
-            )['data']
-        ),
+        days=format_weekly_stats_to_list(stats),
         now=datetime.utcnow()
     )
 
@@ -199,3 +188,21 @@ def calculate_usage(usage):
         'sms_chargeable': max(0, sms_sent - sms_free_allowance),
         'sms_rate': sms_rate
     }
+
+
+def format_weekly_stats_to_list(historical_stats):
+    out = []
+    for week, weekly_stats in historical_stats.items():
+        for stats in weekly_stats.values():
+            stats['failure_rate'] = get_formatted_percentage(stats['failed'], stats['requested'])
+
+        week_start = dateutil.parser.parse(week)
+        week_end = week_start + timedelta(days=6)
+        weekly_stats.update({
+            'week_start': week,
+            'week_end': week_end.date().isoformat(),
+            'week_end_datetime': week_end,
+        })
+        out.append(weekly_stats)
+
+    return sorted(out, key=lambda x: x['week_start'], reverse=True)

app/notify_client/service_api_client.py

@@ -193,6 +193,9 @@ class ServiceAPIClient(NotificationsAPIClient):
     def get_service_usage(self, service_id):
         return self.get('/service/{0}/fragment/aggregate_statistics'.format(service_id))
 
+    def get_weekly_notification_stats(self, service_id):
+        return self.get(url='/service/{}/notifications/weekly'.format(service_id))
+
 
 class ServicesBrowsableItem(BrowsableItem):
     @property

app/notify_client/statistics_api_client.py

@@ -23,17 +23,6 @@ class StatisticsApiClient(BaseAPIClient):
             else:
                 raise e
 
-    def get_7_day_aggregate_for_service(self, service_id, date_from=None, week_count=None):
-        params = {}
-        if date_from is not None:
-            params['date_from'] = date_from
-        if week_count is not None:
-            params['week_count'] = week_count
-        return self.get(
-            url='/service/{}/notifications-statistics/seven_day_aggregate'.format(service_id),
-            params=params
-        )
-
     def get_statistics_for_all_services_for_day(self, day):
         params = {
             'day': day

app/templates/views/weekly.html

@@ -3,7 +3,7 @@
 {% extends "withnav_template.html" %}
 
 {% block page_title %}
-  Daily – GOV.UK Notify
+  Previous weeks – GOV.UK Notify
 {% endblock %}
 
 {% block maincolumn_content %}
@@ -33,16 +33,16 @@
       {% endif %}
     {% endcall %}
     {% call field(align='right') %}
-      {{ item.emails_requested }}
+      {{ item.email.requested }}
     {% endcall %}
     {% call field(align='right') %}
-      {{ item.emails_failure_rate }}%
+      {{ item.email.failure_rate }}%
     {% endcall %}
     {% call field(align='right') %}
-      {{ item.sms_requested }}
+      {{ item.sms.requested }}
     {% endcall %}
     {% call field(align='right') %}
-      {{ item.sms_failure_rate }}%
+      {{ item.sms.failure_rate }}%
     {% endcall %}
   {% endcall %}
 

tests/app/main/views/test_dashboard.py

@@ -1,3 +1,4 @@
+from datetime import datetime
 import copy
 from flask import url_for
 
@@ -5,7 +6,7 @@ import pytest
 from bs4 import BeautifulSoup
 from freezegun import freeze_time
 
-from app.main.views.dashboard import get_dashboard_totals
+from app.main.views.dashboard import get_dashboard_totals, format_weekly_stats_to_list
 
 from tests import validate_route_permission
 from tests.conftest import SERVICE_ONE_ID
@@ -65,7 +66,6 @@ def test_get_started(
     api_user_active,
     mock_get_service,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_aggregate_service_statistics,
     mock_get_user,
     mock_get_user_by_email,
     mock_login,
@@ -94,7 +94,6 @@ def test_get_started_is_hidden_once_templates_exist(
     api_user_active,
     mock_get_service,
     mock_get_service_templates,
-    mock_get_aggregate_service_statistics,
     mock_get_user,
     mock_get_user_by_email,
     mock_login,
@@ -119,7 +118,6 @@ def test_should_show_recent_templates_on_dashboard(app_,
                                                    api_user_active,
                                                    mock_get_service,
                                                    mock_get_service_templates,
-                                                   mock_get_aggregate_service_statistics,
                                                    mock_get_user,
                                                    mock_get_user_by_email,
                                                    mock_login,
@@ -204,7 +202,6 @@ def test_should_show_recent_jobs_on_dashboard(
     api_user_active,
     mock_get_service,
     mock_get_service_templates,
-    mock_get_aggregate_service_statistics,
     mock_get_user,
     mock_get_user_by_email,
     mock_login,
@@ -482,3 +479,46 @@ def test_get_dashboard_totals_adds_warning(failures, expected):
         }
     }
     assert get_dashboard_totals(stats)['sms']['show_warning'] == expected
+
+
+def test_format_weekly_stats_to_list_empty_case():
+    assert format_weekly_stats_to_list({}) == []
+
+
+def test_format_weekly_stats_to_list_sorts_by_week():
+    stats = {
+        '2016-07-04': {},
+        '2016-07-11': {},
+        '2016-07-18': {},
+        '2016-07-25': {}
+    }
+    resp = format_weekly_stats_to_list(stats)
+    assert resp[0]['week_start'] == '2016-07-25'
+    assert resp[1]['week_start'] == '2016-07-18'
+    assert resp[2]['week_start'] == '2016-07-11'
+    assert resp[3]['week_start'] == '2016-07-04'
+
+
+def test_format_weekly_stats_to_list_includes_datetime_for_comparison():
+    stats = {
+        '2016-07-25': {}
+    }
+    resp = format_weekly_stats_to_list(stats)
+    assert resp == [{
+        'week_start': '2016-07-25',
+        'week_end': '2016-07-31',
+        'week_end_datetime': datetime(2016, 7, 31, 0, 0, 0)
+    }]
+
+
+def test_format_weekly_stats_to_list_has_stats_with_failure_rate():
+    stats = {
+        '2016-07-25': {'sms': _stats(3, 1, 2)}
+    }
+    resp = format_weekly_stats_to_list(stats)
+    assert resp[0]['sms']['failure_rate'] == '66.7'
+    assert resp[0]['sms']['requested'] == 3
+
+
+def _stats(requested, delivered, failed):
+    return {'requested': requested, 'delivered': delivered, 'failed': failed}

tests/conftest.py

@@ -220,15 +220,6 @@ def mock_get_service_statistics_for_day(mocker):
         'app.statistics_api_client.get_statistics_for_service_for_day', side_effect=_stats)
 
 
-@pytest.fixture(scope='function')
-def mock_get_aggregate_service_statistics(mocker):
-    def _create(service_id, limit_days=None):
-        return {'data': [{}]}
-
-    return mocker.patch(
-        'app.statistics_api_client.get_7_day_aggregate_for_service', side_effect=_create)
-
-
 @pytest.fixture(scope='function')
 def mock_get_all_service_statistics(mocker):
     def _create(day):