From 83b151982e5826d9c0d7b0a0827e809871447fec Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 25 May 2016 16:51:09 +0100 Subject: [PATCH 1/2] add stats boxes to platform admin page moved a couple of stats summary functions from dashboard to a shared statistics_utils file --- app/main/views/dashboard.py | 51 +-------------------- app/main/views/platform_admin.py | 14 +++++- app/notify_client/statistics_api_client.py | 6 +++ app/statistics_utils.py | 50 ++++++++++++++++++++ app/templates/views/platform-admin.html | 25 ++++++++++ tests/app/main/views/test_platform_admin.py | 25 +++++++++- tests/conftest.py | 9 ++++ 7 files changed, 128 insertions(+), 52 deletions(-) create mode 100644 app/statistics_utils.py diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index c04952809..a5cd84b9b 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -1,8 +1,6 @@ from datetime import datetime, date, timedelta -from dateutil import parser from collections import namedtuple from itertools import groupby -from functools import reduce from flask import ( render_template, @@ -20,7 +18,7 @@ from app import ( service_api_client, template_statistics_client ) - +from app.statistics_utils import sum_of_statistics, add_rates_to from app.utils import user_has_permissions @@ -114,53 +112,6 @@ def weekly(service_id): ) -def sum_of_statistics(delivery_statistics): - - statistics_keys = ( - 'emails_delivered', - 'emails_requested', - 'emails_failed', - 'sms_requested', - 'sms_delivered', - 'sms_failed' - ) - - if not delivery_statistics or not delivery_statistics[0]: - return { - key: 0 for key in statistics_keys - } - - return reduce( - lambda x, y: { - key: x.get(key, 0) + y.get(key, 0) - for key in statistics_keys - }, - delivery_statistics - ) - - -def add_rates_to(delivery_statistics): - - return dict( - emails_failure_rate=( - "{0:.1f}".format( - float(delivery_statistics['emails_failed']) / delivery_statistics['emails_requested'] * 100 - ) - if delivery_statistics['emails_requested'] else 0 - ), - sms_failure_rate=( - "{0:.1f}".format( - float(delivery_statistics['sms_failed']) / delivery_statistics['sms_requested'] * 100 - ) - if delivery_statistics['sms_requested'] else 0 - ), - week_end_datetime=parser.parse( - delivery_statistics.get('week_end', str(datetime.utcnow())) - ), - **delivery_statistics - ) - - def aggregate_usage(template_statistics): immutable_template = namedtuple('Template', ['template_type', 'name', 'id']) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index ef306294b..4a62055a7 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -1,8 +1,13 @@ +from datetime import datetime + +import pytz from flask import render_template from flask_login import login_required +from app import statistics_api_client from app.main import main from app.utils import user_has_permissions +from app.statistics_utils import sum_of_statistics, add_rates_to @main.route("/platform-admin") @@ -10,5 +15,12 @@ from app.utils import user_has_permissions @user_has_permissions(admin_override=True) def platform_admin(): return render_template( - 'views/platform-admin.html' + 'views/platform-admin.html', + global_stats=get_global_stats() ) + + +def get_global_stats(): + day = datetime.now(tz=pytz.timezone('Europe/London')).date() + all_stats = statistics_api_client.get_statistics_for_all_services_for_day(day)['data'] + return add_rates_to(sum_of_statistics(all_stats)) diff --git a/app/notify_client/statistics_api_client.py b/app/notify_client/statistics_api_client.py index 966a4ffc7..33e5a5155 100644 --- a/app/notify_client/statistics_api_client.py +++ b/app/notify_client/statistics_api_client.py @@ -31,3 +31,9 @@ class StatisticsApiClient(BaseAPIClient): 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 + } + return self.get(url='/notifications/statistics', params=params) diff --git a/app/statistics_utils.py b/app/statistics_utils.py new file mode 100644 index 000000000..604b76f1d --- /dev/null +++ b/app/statistics_utils.py @@ -0,0 +1,50 @@ +from datetime import datetime +from dateutil import parser +from functools import reduce + + +def sum_of_statistics(delivery_statistics): + + statistics_keys = ( + 'emails_delivered', + 'emails_requested', + 'emails_failed', + 'sms_requested', + 'sms_delivered', + 'sms_failed' + ) + + if not delivery_statistics or not delivery_statistics[0]: + return { + key: 0 for key in statistics_keys + } + + return reduce( + lambda x, y: { + key: x.get(key, 0) + y.get(key, 0) + for key in statistics_keys + }, + delivery_statistics + ) + + +def add_rates_to(delivery_statistics): + + return dict( + emails_failure_rate=( + "{0:.1f}".format( + float(delivery_statistics['emails_failed']) / delivery_statistics['emails_requested'] * 100 + ) + if delivery_statistics['emails_requested'] else 0 + ), + sms_failure_rate=( + "{0:.1f}".format( + float(delivery_statistics['sms_failed']) / delivery_statistics['sms_requested'] * 100 + ) + if delivery_statistics['sms_requested'] else 0 + ), + week_end_datetime=parser.parse( + delivery_statistics.get('week_end', str(datetime.utcnow())) + ), + **delivery_statistics + ) diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index aab09d9f3..6ad536803 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -1,4 +1,6 @@ {% extends "withoutnav_template.html" %} +{% from "components/big-number.html" import big_number_with_status %} +{% from "components/message-count-label.html" import message_count_label %} {% from "components/browse-list.html" import browse_list %} {% block page_title %} @@ -22,4 +24,27 @@ }, ]) }} + +
+
+ {{ big_number_with_status( + global_stats.emails_delivered, + message_count_label(global_stats.emails_delivered, 'email'), + global_stats.emails_failed, + global_stats.emails_failure_rate, + global_stats.emails_failure_rate|float > 3, + ) }} +
+
+ {{ big_number_with_status( + global_stats.sms_delivered, + message_count_label(global_stats.sms_delivered, 'sms'), + global_stats.sms_failed, + global_stats.sms_failure_rate, + global_stats.sms_failure_rate|float > 3, + ) }} +
+
+ + {% endblock %} diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index e5eb8e7b5..07e65dc55 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -1,7 +1,12 @@ +from datetime import date + from flask import url_for +from freezegun import freeze_time from tests.conftest import mock_get_user +from app.main.views.platform_admin import get_global_stats + def test_should_redirect_if_not_logged_in(app_): with app_.test_request_context(): @@ -22,7 +27,7 @@ def test_should_403_if_not_platform_admin(app_, active_user_with_permissions, mo assert response.status_code == 403 -def test_should_render_platform_admin_page(app_, platform_admin_user, mocker): +def test_should_render_platform_admin_page(app_, platform_admin_user, mocker, mock_get_all_service_statistics): with app_.test_request_context(): with app_.test_client() as client: mock_get_user(mocker, user=platform_admin_user) @@ -34,3 +39,21 @@ def test_should_render_platform_admin_page(app_, platform_admin_user, mocker): assert 'Platform admin' in resp_data assert 'List all services' in resp_data assert 'View providers' in resp_data + + +def test_get_global_stats_should_summarise_all_stats(mock_get_all_service_statistics): + resp = get_global_stats() + + assert 'emails_delivered' in resp + assert 'emails_failed' in resp + assert 'emails_failure_rate' in resp + assert 'sms_delivered' in resp + assert 'sms_failed' in resp + assert 'sms_failure_rate' in resp + + +@freeze_time('2000-06-30T23:30:00', tz_offset=0) +def test_get_global_stats_should_query_for_today_forced_to_GMT(mock_get_all_service_statistics): + get_global_stats() + + mock_get_all_service_statistics.assert_called_once_with(date(2000, 7, 1)) diff --git a/tests/conftest.py b/tests/conftest.py index 037f3814b..298b0272e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -202,6 +202,15 @@ def mock_get_aggregate_service_statistics(mocker): '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): + return {'data': [{}]} + + return mocker.patch( + 'app.statistics_api_client.get_statistics_for_all_services_for_day', side_effect=_create) + + @pytest.fixture(scope='function') def mock_get_service_template(mocker): def _get(service_id, template_id, version=None): From 57875c731b21764d70631e04058088cfb2edd21d Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 26 May 2016 10:16:04 +0100 Subject: [PATCH 2/2] tests for statistics_utils --- tests/app/test_statistics_utils.py | 98 ++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 tests/app/test_statistics_utils.py diff --git a/tests/app/test_statistics_utils.py b/tests/app/test_statistics_utils.py new file mode 100644 index 000000000..5b6c385bc --- /dev/null +++ b/tests/app/test_statistics_utils.py @@ -0,0 +1,98 @@ +import pytest + +from app.statistics_utils import sum_of_statistics, add_rates_to + + +@pytest.mark.parametrize('delivery_statistics', [ + None, + [{}], + [{'emails_requested': 0}, {'emails_requested': 0}] +]) +def test_sum_of_statistics_puts_in_defaults_of_zero(delivery_statistics): + resp = sum_of_statistics(delivery_statistics) + + assert resp == { + 'emails_delivered': 0, + 'emails_requested': 0, + 'emails_failed': 0, + 'sms_requested': 0, + 'sms_delivered': 0, + 'sms_failed': 0 + } + + +def test_sum_of_statistics_sums_inputs(): + delivery_statistics = [ + { + 'emails_delivered': 1, + 'emails_requested': 2, + 'emails_failed': 3, + 'sms_requested': 4, + 'sms_delivered': 5, + 'sms_failed': 6 + }, + { + 'emails_delivered': 10, + 'emails_requested': 20, + 'emails_failed': 30, + 'sms_requested': 40, + 'sms_delivered': 50, + 'sms_failed': 60 + } + ] + resp = sum_of_statistics(delivery_statistics) + + assert resp == { + 'emails_delivered': 11, + 'emails_requested': 22, + 'emails_failed': 33, + 'sms_requested': 44, + 'sms_delivered': 55, + 'sms_failed': 66 + } + + +@pytest.mark.parametrize('emails_failed,emails_requested,expected_failure_rate', [ + (0, 0, 0), + (0, 1, '0.0'), + (1, 3, '33.3') +]) +def test_add_rates_sets_email_failure_rate(emails_failed, emails_requested, expected_failure_rate): + resp = add_rates_to({ + 'emails_failed': emails_failed, + 'emails_requested': emails_requested, + 'sms_failed': 0, + 'sms_requested': 0 + }) + + assert resp['emails_failure_rate'] == expected_failure_rate + + +@pytest.mark.parametrize('sms_failed,sms_requested,expected_failure_rate', [ + (0, 0, 0), + (0, 1, '0.0'), + (1, 3, '33.3') +]) +def test_add_rates_sets_sms_failure_rate(sms_failed, sms_requested, expected_failure_rate): + resp = add_rates_to({ + 'emails_failed': 0, + 'emails_requested': 0, + 'sms_failed': sms_failed, + 'sms_requested': sms_requested + }) + + assert resp['sms_failure_rate'] == expected_failure_rate + + +def test_add_rates_keeps_original_raw_data(): + resp = add_rates_to({ + 'emails_failed': 1, + 'emails_requested': 2, + 'sms_failed': 3, + 'sms_requested': 4 + }) + + assert resp['emails_failed'] == 1 + assert resp['emails_requested'] == 2 + assert resp['sms_failed'] == 3 + assert resp['sms_requested'] == 4