From c88a961e045ffd6ecaf93aa0fe0a000fa66a1b5d Mon Sep 17 00:00:00 2001 From: Jenny Duckett Date: Mon, 5 Dec 2016 12:23:29 +0000 Subject: [PATCH 1/4] Add option to exclude test key stats on platform admin page This page currently includes all notifications for all services, including those sent using a test key. Stats on all other pages exclude test key usage, though, which can lead to confusion for admins comparing numbers between pages. Adding the option to toggle between including and excluding test key usage on the platform admin page gives context for this difference, and allows admins to see live usage of the platform as well as load associated with test key usage. --- app/main/views/platform_admin.py | 12 +++- app/templates/views/platform-admin.html | 62 +++++++++++++-------- tests/app/main/views/test_platform_admin.py | 37 ++++++++++++ 3 files changed, 85 insertions(+), 26 deletions(-) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index a5485b988..d6494620f 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -1,6 +1,9 @@ import itertools -from flask import render_template +from flask import ( + render_template, + request +) from flask_login import login_required from app import service_api_client @@ -13,10 +16,15 @@ from app.statistics_utils import get_formatted_percentage @login_required @user_has_permissions(admin_override=True) def platform_admin(): + include_from_test_key = request.args.get('include_from_test_key') != 'False' # specifically DO get inactive services - services = service_api_client.get_services({'detailed': True})['data'] + api_args = {'detailed': True} + if not include_from_test_key: + api_args['include_from_test_key'] = False + services = service_api_client.get_services(api_args)['data'] return render_template( 'views/platform-admin.html', + include_from_test_key=include_from_test_key, **get_statistics(sorted( services, key=lambda service: (service['active'], service['created_at']), diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index e548cf802..2cc38a372 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -95,31 +95,45 @@ }, ]) }} +
+
+ +
+
+

Today

+
+
+ {{ big_number_with_status( + global_stats.email.delivered, + message_count_label(global_stats.email.delivered, 'email'), + global_stats.email.failed, + global_stats.email.failure_rate, + global_stats.email.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, + ) }} +
+
-

Today

-
-
- {{ big_number_with_status( - global_stats.email.delivered, - message_count_label(global_stats.email.delivered, 'email'), - global_stats.email.failed, - global_stats.email.failure_rate, - global_stats.email.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, - ) }} -
+ {{ services_table(live_services, 'Live services') }} + + {{ services_table(trial_mode_services, 'Trial mode services') }} +
- {{ services_table(live_services, 'Live services') }} - - {{ services_table(trial_mode_services, 'Trial mode services') }} - {% endblock %} diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 77295bedc..eae282364 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -87,6 +87,43 @@ def test_should_render_platform_admin_page( mock_get_detailed_services.assert_called_once_with({'detailed': True}) +@pytest.mark.parametrize('include_from_test_key, expected_text, unexpected_text, api_args', [ + (True, 'Including test key', 'Excluding test key', {'detailed': True}), + (False, 'Excluding test key', 'Including test key', {'detailed': True, 'include_from_test_key': False}) +]) +def test_platform_admin_toggle_including_from_test_key( + include_from_test_key, + expected_text, + unexpected_text, + api_args, + app_, + platform_admin_user, + mocker, + mock_get_detailed_services +): + with app_.test_request_context(): + with app_.test_client() as client: + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + response = client.get(url_for('main.platform_admin', include_from_test_key=str(include_from_test_key))) + + assert response.status_code == 200 + resp_data = response.get_data(as_text=True) + assert expected_text in resp_data + assert unexpected_text not in resp_data + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + change_link = page.find('a', text='change') + assert change_link['href'] + query_param = 'include_from_test_key=False' + if include_from_test_key: + assert query_param in change_link['href'] + else: + assert query_param not in change_link['href'] + + mock_get_detailed_services.assert_called_once_with(api_args) + + def test_create_global_stats_sets_failure_rates(fake_uuid): services = [ service_json(fake_uuid, 'a', []), From 3a637c8fa98254cfbbf531341ff41c65771e50bf Mon Sep 17 00:00:00 2001 From: Jenny Duckett Date: Mon, 5 Dec 2016 17:04:34 +0000 Subject: [PATCH 2/4] Move "Today" heading to left column on platform admin page When we make the numbers on this page more filterable the date range will be one of the options to change, so it makes sense to move it to the side now instead of leaving it above the big numbers. --- app/templates/views/platform-admin.html | 2 +- tests/app/main/views/test_platform_admin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index 2cc38a372..bb774c8de 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -99,6 +99,7 @@
-

Today

{{ big_number_with_status( diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index eae282364..7956b4b7b 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -81,7 +81,7 @@ def test_should_render_platform_admin_page( assert response.status_code == 200 resp_data = response.get_data(as_text=True) assert 'Platform admin' in resp_data - assert 'Today' in resp_data + assert 'Showing stats for today' in resp_data assert 'Live services' in resp_data assert 'Trial mode services' in resp_data mock_get_detailed_services.assert_called_once_with({'detailed': True}) From df265f188d5dcae7d0fb51e7eea528db5d4f98bf Mon Sep 17 00:00:00 2001 From: Jenny Duckett Date: Mon, 5 Dec 2016 17:06:34 +0000 Subject: [PATCH 3/4] Move link to providers page to the header This link looked odd floating above the left column, and although we may want to have admin navigation on the left we aren't sure what that would include yet, so move this link to the header alongside the Platform admin link. --- app/templates/admin_template.html | 3 +++ app/templates/views/platform-admin.html | 8 -------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 14e399d36..ebc1d29cc 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -47,6 +47,9 @@
  • Platform admin
    • +
  • + Providers +
    • {% endif %}
  • Sign out diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index bb774c8de..79c9f750d 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -1,7 +1,6 @@ {% extends "withoutnav_template.html" %} {% from "components/big-number.html" import big_number, big_number_with_status %} {% from "components/message-count-label.html" import message_count_label %} -{% from "components/browse-list.html" import browse_list %} {% from "components/table.html" import mapping_table, field, stats_fields, row_group, row, right_aligned_field_heading, hidden_field_heading, text_field %} {% macro stats_fields(channel, data) -%} @@ -88,13 +87,6 @@ Platform admin - {{ browse_list([ - { - 'title': 'View providers', - 'link': url_for('.view_providers') - }, - ]) }} -