From 26b333512c221e56ac460956ee67a450d40e83bc Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 21 Jul 2017 09:14:29 +0100 Subject: [PATCH] Add separate pages to view live/trial services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a lot of services now. Mostly we want to look at what live services are doing. So loading up the trial mode services every time slows things – generating, rendering and using the page – right down. This commit adds two new pages, one to view only live services, and one to view trial mode services. --- app/main/views/platform_admin.py | 46 ++++++++ .../views/platform-admin/_global_stats.html | 22 ++++ app/templates/views/platform-admin/index.html | 24 +--- .../views/platform-admin/services.html | 108 ++++++++++++++++++ tests/app/main/views/test_platform_admin.py | 54 ++++++--- tests/conftest.py | 23 +++- 6 files changed, 239 insertions(+), 38 deletions(-) create mode 100644 app/templates/views/platform-admin/_global_stats.html create mode 100644 app/templates/views/platform-admin/services.html diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index a6701b5aa..83eb477c2 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -41,6 +41,37 @@ def platform_admin(): ) +@main.route("/platform-admin/live-services", endpoint='live_services') +@main.route("/platform-admin/trial-services", endpoint='trial_services') +@login_required +@user_has_permissions(admin_override=True) +def platform_admin_services(): + form = DateFilterForm(request.args) + api_args = {'detailed': True, # specifically DO get inactive services + 'include_from_test_key': form.include_from_test_key.data + } + + 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() + + services = filter_and_sort_services( + service_api_client.get_services(api_args)['data'], + trial_mode_services=request.endpoint == 'main.trial_services', + ) + + return render_template( + 'views/platform-admin/services.html', + include_from_test_key=form.include_from_test_key.data, + form=form, + services=list(format_stats_by_service(services)), + page_title='{} services'.format( + 'Trial mode' if request.endpoint == 'main.trial_services' else 'Live' + ), + global_stats=create_global_stats(services), + ) + + def sum_service_usage(service): total = 0 for notification_type in service['statistics'].keys(): @@ -60,6 +91,21 @@ def get_statistics(services): } +def filter_and_sort_services(services, trial_mode_services=False): + return ( + service for service in sorted( + services, + key=lambda service: ( + service['active'], + sum_service_usage(service), + service['created_at'] + ), + reverse=True, + ) + if service['restricted'] == trial_mode_services + ) + + def create_global_stats(services): stats = { 'email': { diff --git a/app/templates/views/platform-admin/_global_stats.html b/app/templates/views/platform-admin/_global_stats.html new file mode 100644 index 000000000..e92e17fb2 --- /dev/null +++ b/app/templates/views/platform-admin/_global_stats.html @@ -0,0 +1,22 @@ +{% from "components/big-number.html" import big_number_with_status %} +{% from "components/message-count-label.html" import message_count_label %} +
+
+ {{ big_number_with_status( + global_stats.email.delivered + global_stats.email.failed, + 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 + global_stats.sms.failed, + message_count_label(global_stats.sms.delivered, 'sms'), + global_stats.sms.failed, + global_stats.sms.failure_rate, + global_stats.sms.failure_rate|float > 3, + ) }} +
+
diff --git a/app/templates/views/platform-admin/index.html b/app/templates/views/platform-admin/index.html index 22519751d..46c7e2c96 100644 --- a/app/templates/views/platform-admin/index.html +++ b/app/templates/views/platform-admin/index.html @@ -1,9 +1,8 @@ {% extends "withoutnav_template.html" %} {% from "components/textbox.html" import textbox %} +{% from "components/big-number.html" import big_number %} {% from "components/checkbox.html" import checkbox %} {% from "components/page-footer.html" import page_footer %} -{% from "components/big-number.html" import big_number, big_number_with_status %} -{% from "components/message-count-label.html" import message_count_label %} {% 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) -%} @@ -101,26 +100,7 @@ -
-
- {{ big_number_with_status( - global_stats.email.delivered + global_stats.email.failed, - 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 + global_stats.sms.failed, - message_count_label(global_stats.sms.delivered, 'sms'), - global_stats.sms.failed, - global_stats.sms.failure_rate, - global_stats.sms.failure_rate|float > 3, - ) }} -
-
+ {% include "views/platform-admin/_global_stats.html" %} {{ services_table(live_services, 'Live services') }} diff --git a/app/templates/views/platform-admin/services.html b/app/templates/views/platform-admin/services.html new file mode 100644 index 000000000..f57517543 --- /dev/null +++ b/app/templates/views/platform-admin/services.html @@ -0,0 +1,108 @@ +{% extends "withoutnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/checkbox.html" import checkbox %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/big-number.html" import big_number, big_number_with_status %} +{% from "components/message-count-label.html" import message_count_label %} +{% 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) -%} + + {% call field(border=False) %} + {{ channel.title() }} + {% endcall %} + + {% call field(align='right', border=False) %} + {{ big_number(data[channel]['sending'], smaller=True) }} + {% endcall %} + + {% call field(align='right', border=False) %} + {{ big_number(data[channel]['delivered'], smaller=True) }} + {% endcall %} + + {% call field(align='right', status='error' if data[channel]['failed'], border=False) %} + {{ big_number(data[channel]['failed'], smaller=True) }} + {% endcall %} + +{%- endmacro %} + +{% macro services_table(services, caption) %} + {% call(item, row_number) mapping_table( + caption=caption, + caption_visible=False, + field_headings=[ + 'Service', + hidden_field_heading('Type'), + right_aligned_field_heading('Sending'), + right_aligned_field_heading('Delivered'), + right_aligned_field_heading('Failed') + ], + field_headings_visible=True + ) %} + + {% for service in services %} + + {% call row_group() %} + + {% call row() %} + {% call field(border=False) %} + {{ service['name'] }} + {% endcall %} + + {{ stats_fields('email', service['stats']) }} + {% endcall %} + + {% call row() %} + {% if not service['active'] %} + {% call field(status='default') %} + archived + {% endcall %} + {% elif service['research_mode'] %} + {% call field(border=False) %} + research mode + {% endcall %} + {% elif not service['restricted'] %} + {% call field(status='error') %} + Live + {% endcall %} + {% else %} + {{ text_field('') }} + {% endif %} + + {{ stats_fields('sms', service['stats']) }} + {% endcall %} + + {% endcall %} + + {% endfor %} + + {% endcall %} +{% endmacro %} + + +{% block per_page_title %} + {{ page_title|capitalize }} +{% endblock %} + +{% block maincolumn_content %} + +

+ {{ page_title|capitalize }} +

+ +
+ 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") }} + {{ checkbox(form.include_from_test_key) }} +
+ +
+
+ + {% include "views/platform-admin/_global_stats.html" %} + + {{ services_table(services, page_title|capitalize) }} + +{% endblock %} diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 26a1ac532..9f32bc037 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -10,22 +10,34 @@ from tests import service_json from app.main.views.platform_admin import format_stats_by_service, create_global_stats, sum_service_usage +@pytest.mark.parametrize('endpoint', [ + 'main.platform_admin', + 'main.live_services', + 'main.trial_services', +]) def test_should_redirect_if_not_logged_in( - client + client, + endpoint ): - response = client.get(url_for('main.platform_admin')) + response = client.get(url_for(endpoint)) assert response.status_code == 302 - assert response.location == url_for('main.sign_in', next=url_for('main.platform_admin'), _external=True) + assert response.location == url_for('main.sign_in', next=url_for(endpoint), _external=True) +@pytest.mark.parametrize('endpoint', [ + 'main.platform_admin', + 'main.live_services', + 'main.trial_services', +]) def test_should_403_if_not_platform_admin( client, active_user_with_permissions, mocker, + endpoint, ): mock_get_user(mocker, user=active_user_with_permissions) client.login(active_user_with_permissions) - response = client.get(url_for('main.platform_admin')) + response = client.get(url_for(endpoint)) assert response.status_code == 403 @@ -64,24 +76,33 @@ def test_should_show_research_and_restricted_mode( assert service_mode == displayed +@pytest.mark.parametrize('endpoint, expected_services_shown', [ + ('main.platform_admin', 2), + ('main.live_services', 1), + ('main.trial_services', 1), +]) def test_should_render_platform_admin_page( client, platform_admin_user, mocker, mock_get_detailed_services, + endpoint, + expected_services_shown, ): mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) - response = client.get(url_for('main.platform_admin')) - + response = client.get(url_for(endpoint)) assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Platform admin' in resp_data - assert 'Live services' in resp_data - assert 'Trial mode services' in resp_data + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert len(page.select('tbody tr')) == expected_services_shown * 2 # one row for SMS, one for email mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) +@pytest.mark.parametrize('endpoint', [ + 'main.platform_admin', + 'main.live_services', + 'main.trial_services', +]) @pytest.mark.parametrize('include_from_test_key, api_args', [ ("Y", {'detailed': True, 'include_from_test_key': True}), ("N", {'detailed': True, 'include_from_test_key': False}) @@ -93,30 +114,35 @@ def test_platform_admin_toggle_including_from_test_key( platform_admin_user, mocker, mock_get_detailed_services, + endpoint, ): 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=include_from_test_key)) + response = client.get(url_for(endpoint, include_from_test_key=include_from_test_key)) assert response.status_code == 200 mock_get_detailed_services.assert_called_once_with(api_args) +@pytest.mark.parametrize('endpoint', [ + 'main.platform_admin', + 'main.live_services', + 'main.trial_services', +]) def test_platform_admin_with_date_filter( client, platform_admin_user, mocker, mock_get_detailed_services, + endpoint, ): mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) - response = client.get(url_for('main.platform_admin', start_date='2016-12-20', end_date='2016-12-28')) + response = client.get(url_for(endpoint, start_date='2016-12-20', end_date='2016-12-28')) assert response.status_code == 200 resp_data = response.get_data(as_text=True) assert 'Platform admin' in resp_data - assert 'Live services' in resp_data - assert 'Trial mode services' in resp_data mock_get_detailed_services.assert_called_once_with({ 'include_from_test_key': False, 'start_date': datetime.date(2016, 12, 20), diff --git a/tests/conftest.py b/tests/conftest.py index c3f83457e..273b9d181 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -121,12 +121,31 @@ def mock_get_detailed_service_for_today(mocker, api_user_active): @pytest.fixture(scope='function') def mock_get_detailed_services(mocker, fake_uuid): - service_one = service_json(SERVICE_ONE_ID, "service_one", [fake_uuid], 1000, True, False) + service_one = service_json( + id_=SERVICE_ONE_ID, + name="service_one", + users=[fake_uuid], + message_limit=1000, + active=True, + restricted=False, + ) + service_two = service_json( + id_=fake_uuid, + name="service_two", + users=[fake_uuid], + message_limit=1000, + active=True, + restricted=True, + ) service_one['statistics'] = { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} } - services = {'data': [service_one]} + service_two['statistics'] = { + 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} + } + services = {'data': [service_one, service_two]} return mocker.patch('app.service_api_client.get_services', return_value=services)