diff --git a/app/__init__.py b/app/__init__.py index ee6a4a899..d3f74b9b5 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -59,6 +59,7 @@ from app.notify_client.letter_jobs_client import LetterJobsClient from app.notify_client.inbound_number_client import InboundNumberClient from app.notify_client.billing_api_client import BillingAPIClient from app.notify_client.complaint_api_client import ComplaintApiClient +from app.notify_client.platform_stats_api_client import PlatformStatsAPIClient from app.commands import setup_commands from app.utils import get_cdn_domain from app.utils import gmt_timezones @@ -86,6 +87,7 @@ letter_jobs_client = LetterJobsClient() inbound_number_client = InboundNumberClient() billing_api_client = BillingAPIClient() complaint_api_client = ComplaintApiClient() +platform_stats_api_client = PlatformStatsAPIClient() # The current service attached to the request stack. current_service = LocalProxy(partial(_lookup_req_object, 'service')) @@ -131,6 +133,7 @@ def create_app(application): inbound_number_client.init_app(application) billing_api_client.init_app(application) complaint_api_client.init_app(application) + platform_stats_api_client.init_app(application) login_manager.init_app(application) login_manager.login_view = 'main.sign_in' diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index 7bc54d38f..0e64b1e23 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -251,3 +251,9 @@ details .arrow { cursor: pointer; } } + +.bordered-text-box { + padding: 5px; + outline: 2px solid $black; + max-width: 100%; +} diff --git a/app/assets/stylesheets/components/big-number.scss b/app/assets/stylesheets/components/big-number.scss index 57590d461..7e4fa81a8 100644 --- a/app/assets/stylesheets/components/big-number.scss +++ b/app/assets/stylesheets/components/big-number.scss @@ -16,6 +16,18 @@ } +.big-number-dark { + @extend %big-number; + padding: $gutter-half; + position: relative; + background: $black; + color: $white; + + .big-number-number { + @include bold-36($tabular-numbers: true); + } +} + .big-number-smaller { @extend %big-number; diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 6321d862d..c4164ce10 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -1,15 +1,23 @@ import itertools from datetime import datetime -from flask import render_template, request +from flask import render_template, request, url_for from flask_login import login_required -from app import complaint_api_client, service_api_client +from app import ( + complaint_api_client, + platform_stats_api_client, + service_api_client, +) from app.main import main from app.main.forms import DateFilterForm from app.statistics_utils import get_formatted_percentage from app.utils import user_is_platform_admin +COMPLAINT_THRESHOLD = 0.02 +FAILURE_THRESHOLD = 3 +ZERO_FAILURE_THRESHOLD = 0 + @main.route("/platform-admin") @login_required @@ -38,6 +46,114 @@ def platform_admin(): ) +@main.route("/platform-admin-new") +@login_required +@user_is_platform_admin +def platform_admin_new(): + form = DateFilterForm(request.args, meta={'csrf': False}) + api_args = {} + + form.validate() + + if form.start_date.data: + api_args['start_date'] = form.start_date.data + api_args['end_date'] = form.end_date.data or datetime.utcnow().date() + + platform_stats = platform_stats_api_client.get_aggregate_platform_stats(api_args) + number_of_complaints = complaint_api_client.get_complaint_count(api_args) + + return render_template( + 'views/platform-admin/index_new.html', + form=form, + global_stats=make_columns(platform_stats, number_of_complaints) + ) + + +def is_over_threshold(number, total, threshold): + percentage = number / total * 100 if total else 0 + return percentage > threshold + + +def get_status_box_data(stats, key, label, threshold=FAILURE_THRESHOLD): + return { + 'number': stats['failures'][key], + 'label': label, + 'failing': is_over_threshold( + stats['failures'][key], + stats['total'], + threshold + ), + 'percentage': get_formatted_percentage(stats['failures'][key], stats['total']) + } + + +def get_tech_failure_status_box_data(stats): + stats = get_status_box_data(stats, 'technical-failure', 'technical failures', ZERO_FAILURE_THRESHOLD) + stats.pop('percentage') + return stats + + +def make_columns(global_stats, complaints_number): + return [ + # email + { + 'black_box': { + 'number': global_stats['email']['total'], + 'notification_type': 'email' + }, + 'other_data': [ + get_tech_failure_status_box_data(global_stats['email']), + get_status_box_data(global_stats['email'], 'permanent-failure', 'permanent failures'), + get_status_box_data(global_stats['email'], 'temporary-failure', 'temporary failures'), + { + 'number': complaints_number, + 'label': 'complaints', + 'failing': is_over_threshold(complaints_number, + global_stats['email']['total'], COMPLAINT_THRESHOLD), + 'percentage': get_formatted_percentage(complaints_number, global_stats['email']['total']), + 'url': url_for('main.platform_admin_list_complaints') + } + ], + 'test_data': { + 'number': global_stats['email']['test-key'], + 'label': 'test emails' + } + }, + # sms + { + 'black_box': { + 'number': global_stats['sms']['total'], + 'notification_type': 'sms' + }, + 'other_data': [ + get_tech_failure_status_box_data(global_stats['sms']), + get_status_box_data(global_stats['sms'], 'permanent-failure', 'permanent failures'), + get_status_box_data(global_stats['sms'], 'temporary-failure', 'temporary failures') + ], + 'test_data': { + 'number': global_stats['sms']['test-key'], + 'label': 'test text messages' + } + }, + # letter + { + 'black_box': { + 'number': global_stats['letter']['total'], + 'notification_type': 'letter' + }, + 'other_data': [ + get_tech_failure_status_box_data(global_stats['letter']), + get_status_box_data(global_stats['letter'], + 'virus-scan-failed', 'virus scan failures', ZERO_FAILURE_THRESHOLD) + ], + 'test_data': { + 'number': global_stats['letter']['test-key'], + 'label': 'test letters' + } + }, + ] + + @main.route("/platform-admin/live-services", endpoint='live_services') @main.route("/platform-admin/trial-services", endpoint='trial_services') @login_required diff --git a/app/navigation.py b/app/navigation.py index 2725f50ca..0b5f8bfc3 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -79,6 +79,7 @@ class HeaderNavigation(Navigation): 'live_services', 'organisations', 'platform_admin', + 'platform_admin_new', 'suspend_service', 'trial_services', 'update_email_branding', @@ -421,6 +422,7 @@ class MainNavigation(Navigation): 'organisation_settings', 'organisations', 'platform_admin', + 'platform_admin_new', 'platform_admin_list_complaints', 'pricing', 'privacy', @@ -590,6 +592,7 @@ class OrgNavigation(Navigation): 'old_using_notify', 'organisations', 'platform_admin', + 'platform_admin_new', 'platform_admin_list_complaints', 'pricing', 'privacy', diff --git a/app/notify_client/complaint_api_client.py b/app/notify_client/complaint_api_client.py index 6e657396c..72760d0b7 100644 --- a/app/notify_client/complaint_api_client.py +++ b/app/notify_client/complaint_api_client.py @@ -9,3 +9,6 @@ class ComplaintApiClient(NotifyAdminAPIClient): def get_all_complaints(self): return self.get('/complaint') + + def get_complaint_count(self, params_dict=None): + return self.get('/complaint/count-by-date-range', params=params_dict) diff --git a/app/notify_client/platform_stats_api_client.py b/app/notify_client/platform_stats_api_client.py new file mode 100644 index 000000000..9ca9390de --- /dev/null +++ b/app/notify_client/platform_stats_api_client.py @@ -0,0 +1,11 @@ +from app.notify_client import NotifyAdminAPIClient + + +class PlatformStatsAPIClient(NotifyAdminAPIClient): + # Fudge assert in the super __init__ so + # we can set those variables later. + def __init__(self): + super().__init__("a" * 73, "b") + + def get_aggregate_platform_stats(self, params_dict=None): + return self.get("/platform-stats", params=params_dict) diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index 454a0cc05..b5db30d5b 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -55,3 +55,19 @@ {% endif %} {% endmacro %} + + +{% macro big_number_simple(number, label) %} +
+
+ {% if number is number %} + {{ "{:,}".format(number) }} + {% else %} + {{ number }} + {% endif %} +
+ {% if label %} + {{ label }} + {% endif %} +
+{% endmacro %} diff --git a/app/templates/components/status-box.html b/app/templates/components/status-box.html new file mode 100644 index 000000000..257430e2b --- /dev/null +++ b/app/templates/components/status-box.html @@ -0,0 +1,15 @@ +{% macro status_box(number, label, failing=false, percentage=None, url=None) %} +
+
+ {% if url %} + {{ number }} {{ label }} + {% else %} + {{ number }} {{ label }} + {% endif %} + + {% if percentage %} + - {{ percentage }}% + {% endif %} +
+
+{% endmacro %} diff --git a/app/templates/views/platform-admin/index_new.html b/app/templates/views/platform-admin/index_new.html new file mode 100644 index 000000000..4754af1b0 --- /dev/null +++ b/app/templates/views/platform-admin/index_new.html @@ -0,0 +1,58 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/big-number.html" import big_number_simple %} +{% from "components/message-count-label.html" import message_count_label %} +{% from "components/status-box.html" import status_box %} + +{% block per_page_title %} + Platform admin +{% endblock %} + +{% block platform_admin_content %} + +

+ Summary +

+
+ Apply filters +
+ {{ textbox(form.start_date, hint="Enter start date in format YYYY-MM-DD") }} + {{ textbox(form.end_date, hint="Enter end date in format YYYY-MM-DD") }} +
+ +
+
+ +
+ {% for noti_type in global_stats %} +
+ {{ big_number_simple( + noti_type.black_box.number, + message_count_label(noti_type.black_box.number, noti_type.black_box.notification_type) + ) }} + + {% for item in noti_type.other_data %} + {{ status_box( + number=item.number, + label=item.label, + failing=item.failing, + percentage=item.percentage, + url=item.url) + }} + {% endfor %} +
+ {% endfor %} +
+ +
+ {% for noti_type in global_stats %} +
+
+ {{ "{:,}".format(noti_type.test_data.number) }} + {{ noti_type.test_data.label }} +
+
+ {% endfor %} +
+ +{% endblock %} diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index dd0d47f13..8c587a23e 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -1,14 +1,18 @@ import datetime +import re import uuid from unittest.mock import ANY import pytest from bs4 import BeautifulSoup from flask import url_for +from freezegun import freeze_time from app.main.views.platform_admin import ( create_global_stats, format_stats_by_service, + get_tech_failure_status_box_data, + is_over_threshold, sum_service_usage, ) from tests import service_json @@ -667,3 +671,114 @@ def test_platform_admin_list_complaints( resp_data = response.get_data(as_text=True) assert 'Email complaints' in resp_data assert mock.called + + +@pytest.mark.parametrize('number, total, threshold, result', [ + (0, 0, 0, False), + (1, 1, 0, True), + (2, 3, 66, True), + (2, 3, 67, False), +]) +def test_is_over_threshold(number, total, threshold, result): + assert is_over_threshold(number, total, threshold) is result + + +def test_get_tech_failure_status_box_data_removes_percentage_data(): + stats = { + 'failures': + {'permanent-failure': 0, 'technical-failure': 0, 'temporary-failure': 1, 'virus-scan-failed': 0}, + 'test-key': 0, + 'total': 5589 + } + tech_failure_data = get_tech_failure_status_box_data(stats) + + assert 'percentage' not in tech_failure_data + + +def test_platform_admin_new_with_start_and_end_dates_provided(mocker, logged_in_platform_admin_client): + start_date = '2018-01-01' + end_date = '2018-06-01' + api_args = {'start_date': datetime.date(2018, 1, 1), 'end_date': datetime.date(2018, 6, 1)} + + mocker.patch('app.main.views.platform_admin.make_columns') + aggregate_stats_mock = mocker.patch( + 'app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats') + complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') + + logged_in_platform_admin_client.get( + url_for('main.platform_admin_new', start_date=start_date, end_date=end_date) + ) + + aggregate_stats_mock.assert_called_with(api_args) + complaint_count_mock.assert_called_with(api_args) + + +@freeze_time('2018-6-11') +def test_platform_admin_new_with_only_a_start_date_provided(mocker, logged_in_platform_admin_client): + start_date = '2018-01-01' + api_args = {'start_date': datetime.date(2018, 1, 1), 'end_date': datetime.datetime.utcnow().date()} + + mocker.patch('app.main.views.platform_admin.make_columns') + aggregate_stats_mock = mocker.patch( + 'app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats') + complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') + + logged_in_platform_admin_client.get(url_for('main.platform_admin_new', start_date=start_date)) + + aggregate_stats_mock.assert_called_with(api_args) + complaint_count_mock.assert_called_with(api_args) + + +def test_platform_admin_new_without_dates_provided(mocker, logged_in_platform_admin_client): + api_args = {} + + mocker.patch('app.main.views.platform_admin.make_columns') + aggregate_stats_mock = mocker.patch( + 'app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats') + complaint_count_mock = mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') + + logged_in_platform_admin_client.get(url_for('main.platform_admin_new')) + + aggregate_stats_mock.assert_called_with(api_args) + complaint_count_mock.assert_called_with(api_args) + + +def test_platform_admin_new_displays_stats_in_right_boxes_and_with_correct_styling( + mocker, + logged_in_platform_admin_client, +): + platform_stats = { + 'email': {'failures': + {'permanent-failure': 3, 'technical-failure': 0, 'temporary-failure': 0, 'virus-scan-failed': 0}, + 'test-key': 0, + 'total': 145}, + 'sms': {'failures': + {'permanent-failure': 0, 'technical-failure': 1, 'temporary-failure': 0, 'virus-scan-failed': 0}, + 'test-key': 5, + 'total': 168}, + 'letter': {'failures': + {'permanent-failure': 0, 'technical-failure': 0, 'temporary-failure': 1, 'virus-scan-failed': 1}, + 'test-key': 0, + 'total': 500} + } + mocker.patch('app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats', + return_value=platform_stats) + mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count', return_value=15) + + response = logged_in_platform_admin_client.get(url_for('main.platform_admin_new')) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + # Email permanent failure status box - number is correct + assert '3 permanent failures' in page.find_all('div', class_='column-third')[0].find(string=re.compile('permanent')) + # Email complaints status box - link exists and number is correct + assert page.find('a', string='15 complaints') + # SMS total box - number is correct + assert page.find_all('div', class_='big-number-number')[1].text.strip() == '168' + # Test SMS box - number is correct + assert '5' in page.find_all('div', class_='column-third')[4].text + # SMS technical failure status box - number is correct and failure class is used + assert '1 technical failures' in page.find_all('div', class_='column-third')[1].find( + 'div', class_='big-number-status-failing').text + # Letter virus scan failure status box - number is correct and failure class is used + assert '1 virus scan failures' in page.find_all('div', class_='column-third')[2].find( + 'div', class_='big-number-status-failing').text diff --git a/tests/app/notify_client/test_compliant_client.py b/tests/app/notify_client/test_compliant_client.py index 9d05d1490..2665a14a7 100644 --- a/tests/app/notify_client/test_compliant_client.py +++ b/tests/app/notify_client/test_compliant_client.py @@ -8,3 +8,12 @@ def test_get_all_complaints(mocker): client.get_all_complaints() mock.assert_called_once_with('/complaint') + + +def test_get_complaint_count(mocker): + client = ComplaintApiClient() + mock = mocker.patch.object(client, 'get') + params_dict = {'start_date': '2018-06-01', 'end_date': '2018-06-15'} + + client.get_complaint_count(params_dict=params_dict) + mock.assert_called_once_with('/complaint/count-by-date-range', params=params_dict) diff --git a/tests/app/notify_client/test_platform_stats_api_client.py b/tests/app/notify_client/test_platform_stats_api_client.py new file mode 100644 index 000000000..e0f510c86 --- /dev/null +++ b/tests/app/notify_client/test_platform_stats_api_client.py @@ -0,0 +1,10 @@ +from app.notify_client.platform_stats_api_client import PlatformStatsAPIClient + + +def test_get_aggregate_platform_stats(mocker): + client = PlatformStatsAPIClient() + mock = mocker.patch.object(client, 'get') + params_dict = {'start_date': '2018-06-01', 'end_date': '2018-06-15'} + + client.get_aggregate_platform_stats(params_dict=params_dict) + mock.assert_called_once_with('/platform-stats', params=params_dict)