diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index bfc8cb94b..d0a4b5c07 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -30,14 +30,41 @@ def platform_admin(): services = service_api_client.get_services(api_args)['data'] return render_template( - 'views/platform-admin.html', + 'views/platform-admin/index.html', include_from_test_key=form.include_from_test_key.data, form=form, - **get_statistics(sorted( - services, - key=lambda service: (service['active'], sum_service_usage(service), service['created_at']), - reverse=True - )) + global_stats=create_global_stats(services), + ) + + +@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), ) @@ -48,16 +75,19 @@ def sum_service_usage(service): return total -def get_statistics(services): - return { - 'global_stats': create_global_stats(services), - 'live_services': format_stats_by_service( - service for service in services if not service['restricted'] - ), - 'trial_mode_services': format_stats_by_service( - service for service in services if service['restricted'] - ), - } +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): diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index cd00e56a2..215563caa 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -49,12 +49,6 @@
  • Platform admin
    • -
  • - Providers -
    • -
  • - Letter jobs -
    • {% endif %}
  • Sign out diff --git a/app/templates/views/letter-jobs.html b/app/templates/views/letter-jobs.html index 053a813af..d314acfa0 100644 --- a/app/templates/views/letter-jobs.html +++ b/app/templates/views/letter-jobs.html @@ -1,11 +1,11 @@ -{% extends "withoutnav_template.html" %} +{% extends "views/platform-admin/_base_template.html" %} {% from "components/page-footer.html" import page_footer %} {% block per_page_title %} Letter jobs {% endblock %} -{% block maincolumn_content %} +{% block platform_admin_content %}

    Letter jobs

    diff --git a/app/templates/views/platform-admin/_base_template.html b/app/templates/views/platform-admin/_base_template.html new file mode 100644 index 000000000..d48756cab --- /dev/null +++ b/app/templates/views/platform-admin/_base_template.html @@ -0,0 +1,34 @@ +{% extends "withoutnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/checkbox.html" import checkbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block maincolumn_content %} + +
    +
    +

    + Platform admin +

    + +
    +
    + {% block platform_admin_content %}{% endblock %} +
    +
    + +{% endblock %} 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 new file mode 100644 index 000000000..9a3f0f17e --- /dev/null +++ b/app/templates/views/platform-admin/index.html @@ -0,0 +1,27 @@ +{% extends "views/platform-admin/_base_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/table.html" import mapping_table, field, stats_fields, row_group, row, right_aligned_field_heading, hidden_field_heading, text_field %} + +{% block per_page_title %} + Platform admin +{% endblock %} + +{% block platform_admin_content %} + +
    + 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" %} + +{% endblock %} diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin/services.html similarity index 75% rename from app/templates/views/platform-admin.html rename to app/templates/views/platform-admin/services.html index 22519751d..d866658d1 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin/services.html @@ -1,4 +1,4 @@ -{% extends "withoutnav_template.html" %} +{% extends "views/platform-admin/_base_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/checkbox.html" import checkbox %} {% from "components/page-footer.html" import page_footer %} @@ -29,7 +29,7 @@ {% macro services_table(services, caption) %} {% call(item, row_number) mapping_table( caption=caption, - caption_visible=True, + caption_visible=False, field_headings=[ 'Service', hidden_field_heading('Type'), @@ -81,13 +81,13 @@ {% block per_page_title %} - Platform admin + {{ page_title|capitalize }} {% endblock %} -{% block maincolumn_content %} +{% block platform_admin_content %}

    - Platform admin + {{ page_title|capitalize }}

    @@ -101,29 +101,8 @@
    -
    -
    - {{ 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') }} - - {{ services_table(trial_mode_services, 'Trial mode services') }} + {{ services_table(services, page_title|capitalize) }} {% endblock %} diff --git a/app/templates/views/providers/providers.html b/app/templates/views/providers/providers.html index 2da7cde9d..b7320fa87 100644 --- a/app/templates/views/providers/providers.html +++ b/app/templates/views/providers/providers.html @@ -1,4 +1,4 @@ -{% extends "withoutnav_template.html" %} +{% extends "views/platform-admin/_base_template.html" %} {% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading %} {% from "components/show-more.html" import show_more %} @@ -6,112 +6,107 @@ Providers {% endblock %} -{% block maincolumn_content %} +{% block platform_admin_content %} -
    -
    -

    Providers

    +

    Providers

    -

    SMS

    +

    SMS

    - {% call(item, row_number) list_table( - domestic_sms_providers, - caption="Domestic SMS providers", - caption_visible=False, - empty_message='No domestic sms providers', - field_headings=['Provider', 'Priority', 'Active', 'Last Updated', 'Updated By'], - field_headings_visible=True - ) %} + {% call(item, row_number) list_table( + domestic_sms_providers, + caption="Domestic SMS providers", + caption_visible=False, + empty_message='No domestic sms providers', + field_headings=['Provider', 'Priority', 'Active', 'Last Updated', 'Updated By'], + field_headings_visible=True + ) %} - {{ link_field(item.display_name, url_for('main.view_provider', provider_id=item.id)) }} + {{ link_field(item.display_name, url_for('main.view_provider', provider_id=item.id)) }} - {{ text_field(item.priority) }} + {{ text_field(item.priority) }} - {{ text_field(item.active) }} + {{ text_field(item.active) }} - {% if item.updated_at %} - {{ text_field(item.updated_at|format_datetime_short) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {% if item.updated_at %} + {{ text_field(item.updated_at|format_datetime_short) }} + {% else %} + {{ text_field('None') }} + {% endif %} - {% if item.created_by %} - {{ text_field(item.created_by.name) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {% if item.created_by %} + {{ text_field(item.created_by.name) }} + {% else %} + {{ text_field('None') }} + {% endif %} - {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} + {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} - {% endcall %} + {% endcall %} -

    Email

    +

    Email

    - {% call(item, row_number) list_table( - email_providers, - caption="Email providers", - caption_visible=False, - empty_message='No email providers', - field_headings=['Provider', 'Priority', 'Active', 'Last Updated', 'Updated By'], - field_headings_visible=True - ) %} + {% call(item, row_number) list_table( + email_providers, + caption="Email providers", + caption_visible=False, + empty_message='No email providers', + field_headings=['Provider', 'Priority', 'Active', 'Last Updated', 'Updated By'], + field_headings_visible=True + ) %} - {{ link_field(item.display_name, url_for('main.view_provider', provider_id=item.id)) }} + {{ link_field(item.display_name, url_for('main.view_provider', provider_id=item.id)) }} - {{ text_field(item.priority) }} + {{ text_field(item.priority) }} - {{ text_field(item.active) }} + {{ text_field(item.active) }} - {% if item.updated_at %} - {{ text_field(item.updated_at|format_datetime_short) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {% if item.updated_at %} + {{ text_field(item.updated_at|format_datetime_short) }} + {% else %} + {{ text_field('None') }} + {% endif %} - {% if item.created_by %} - {{ text_field(item.created_by.name) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {% if item.created_by %} + {{ text_field(item.created_by.name) }} + {% else %} + {{ text_field('None') }} + {% endif %} - {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} + {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} - {% endcall %} + {% endcall %} -

    International SMS Providers

    +

    International SMS Providers

    - {% call(item, row_number) list_table( - intl_sms_providers, - caption="International SMS providers", - caption_visible=False, - empty_message='No international sms providers', - field_headings=['Provider', 'Priority', 'Active', 'Last Updated', 'Updated By'], - field_headings_visible=True - ) %} + {% call(item, row_number) list_table( + intl_sms_providers, + caption="International SMS providers", + caption_visible=False, + empty_message='No international sms providers', + field_headings=['Provider', 'Priority', 'Active', 'Last Updated', 'Updated By'], + field_headings_visible=True + ) %} - {{ link_field(item.display_name, url_for('main.view_provider', provider_id=item.id)) }} + {{ link_field(item.display_name, url_for('main.view_provider', provider_id=item.id)) }} - {{ text_field(item.priority) }} + {{ text_field(item.priority) }} - {{ text_field(item.active) }} + {{ text_field(item.active) }} - {% if item.updated_at %} - {{ text_field(item.updated_at|format_datetime_short) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {% if item.updated_at %} + {{ text_field(item.updated_at|format_datetime_short) }} + {% else %} + {{ text_field('None') }} + {% endif %} - {% if item.created_by %} - {{ text_field(item.created_by.name) }} - {% else %} - {{ text_field('None') }} - {% endif %} + {% if item.created_by %} + {{ text_field(item.created_by.name) }} + {% else %} + {{ text_field('None') }} + {% endif %} - {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} + {{ link_field('change', url_for('main.edit_provider', provider_id=item.id)) }} - {% endcall %} - -
    -
    + {% endcall %} {% endblock %} diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 26a1ac532..6c1872ef2 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -10,35 +10,47 @@ 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 -@pytest.mark.parametrize('restricted, table_index, research_mode, displayed', [ - (True, 1, False, ''), - (False, 0, False, 'Live'), - (False, 0, True, 'research mode'), - (True, 1, True, 'research mode') +@pytest.mark.parametrize('endpoint, restricted, research_mode, displayed', [ + ('main.trial_services', True, False, ''), + ('main.live_services', False, False, 'Live'), + ('main.live_services', False, True, 'research mode'), + ('main.trial_services', True, True, 'research mode') ]) def test_should_show_research_and_restricted_mode( + endpoint, restricted, - table_index, research_mode, displayed, client, @@ -53,35 +65,43 @@ def test_should_show_research_and_restricted_mode( mock_get_detailed_services.return_value = {'data': services} 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 mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') # get first column in second row, which contains flags as text. - table_body = page.find_all('table')[table_index].find_all('tbody')[0] + table_body = page.find_all('table')[0].find_all('tbody')[0] service_mode = table_body.find_all('tbody')[0].find_all('tr')[1].find_all('td')[0].text.strip() assert service_mode == displayed +@pytest.mark.parametrize('endpoint, expected_services_shown', [ + ('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 +113,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), @@ -203,13 +228,13 @@ def test_format_stats_by_service_returns_correct_values(fake_uuid): assert ret[0]['stats']['sms']['failed'] == 11 -@pytest.mark.parametrize('restricted, table_index, research_mode', [ - (True, 1, False), - (False, 0, False) +@pytest.mark.parametrize('endpoint, restricted, research_mode', [ + ('main.trial_services', True, False), + ('main.live_services', False, False) ]) def test_should_show_email_and_sms_stats_for_all_service_types( + endpoint, restricted, - table_index, research_mode, client, platform_admin_user, @@ -230,13 +255,13 @@ def test_should_show_email_and_sms_stats_for_all_service_types( mock_get_detailed_services.return_value = {'data': services} 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 mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - table_body = page.find_all('table')[table_index].find_all('tbody')[0] + table_body = page.find_all('table')[0].find_all('tbody')[0] service_row_group = table_body.find_all('tbody')[0].find_all('tr') email_stats = service_row_group[0].find_all('div', class_='big-number-number') sms_stats = service_row_group[1].find_all('div', class_='big-number-number') @@ -251,17 +276,17 @@ def test_should_show_email_and_sms_stats_for_all_service_types( assert sms_failed == 11 -@pytest.mark.parametrize('restricted, table_index', [ - (False, 0), - (True, 1) +@pytest.mark.parametrize('endpoint, restricted', [ + ('main.live_services', False), + ('main.trial_services', True) ], ids=['live', 'trial']) def test_should_show_archived_services_last( + endpoint, client, platform_admin_user, mocker, mock_get_detailed_services, restricted, - table_index, ): services = [ service_json(name='C', restricted=restricted, active=False, created_at='2002-02-02 12:00:00'), @@ -275,13 +300,13 @@ def test_should_show_archived_services_last( mock_get_detailed_services.return_value = {'data': services} 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 mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - table_body = page.find_all('table')[table_index].find_all('tbody')[0] + table_body = page.find_all('table')[0].find_all('tbody')[0] services = [service.tr for service in table_body.find_all('tbody')] assert len(services) == 3 assert services[0].td.text.strip() == 'A' @@ -305,7 +330,7 @@ def test_shows_archived_label_instead_of_live_or_research_mode_label( mock_get_detailed_services.return_value = {'data': services} 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('main.live_services')) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -349,13 +374,13 @@ def test_should_show_correct_sent_totals_for_platform_admin( assert sms_total == 40 -@pytest.mark.parametrize('restricted, table_index, research_mode', [ - (True, 1, False), - (False, 0, False) +@pytest.mark.parametrize('endpoint, restricted, research_mode', [ + ('main.trial_services', True, False), + ('main.live_services', False, False) ]) def test_should_order_services_by_usage_with_inactive_last( + endpoint, restricted, - table_index, research_mode, client, platform_admin_user, @@ -398,13 +423,13 @@ def test_should_order_services_by_usage_with_inactive_last( mock_get_detailed_services.return_value = {'data': services} 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 mock_get_detailed_services.assert_called_once_with({'detailed': True, 'include_from_test_key': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - table_body = page.find_all('table')[table_index].find_all('tbody')[0] + table_body = page.find_all('table')[0].find_all('tbody')[0] services = [service.tr for service in table_body.find_all('tbody')] assert len(services) == 3 assert services[0].td.text.strip() == 'My Service 2' 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)