From 21560858131fa54b2cd22cacd6b9b7cd97502616 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Sep 2016 11:30:25 +0100 Subject: [PATCH 1/4] Fix right aligned table headings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CSS for these wasn’t being set on the correct class. --- app/assets/stylesheets/components/table.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 1d6c3bbeb..ccb9269d2 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -135,6 +135,10 @@ width: 52.5%; } +} + +.table-field-heading { + &:last-child { padding-right: 0; } From 29d5bc4f51e4f40eece65ff2131ee39cbdf09a59 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Sep 2016 11:33:30 +0100 Subject: [PATCH 2/4] Factor services table into a macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we’re going to have spearate tables for live and trial mode services it saves copying and pasting to have a macro for them. Not worth completely factoring out into a component because it’s only going to be used on this page. --- app/templates/views/platform-admin.html | 88 +++++++++++++------------ 1 file changed, 47 insertions(+), 41 deletions(-) diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index f8c93e534..519ecb4ec 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -4,6 +4,51 @@ {% from "components/browse-list.html" import browse_list %} {% from "components/table.html" import list_table, field, right_aligned_field_heading, hidden_field_heading, text_field %} +{% macro services_table(services, caption) %} + {% call(item, row_number) list_table( + services, + caption=caption, + caption_visible=False, + field_headings=[ + 'Service', + hidden_field_heading('Status'), + right_aligned_field_heading('Sending'), + right_aligned_field_heading('Delivered'), + right_aligned_field_heading('Failed') + ], + field_headings_visible=True + ) %} + {% call field() %} +
+ {{ item['name'] }} +
+ {% endcall %} + {% if item['research_mode'] %} + {% call field() %} + research mode + {% endcall %} + {% elif not item['restricted'] %} + {% call field(status='error') %} + + Live + + {% endcall %} + {% else %} + {{ text_field('') }} + {% endif %} + {% call field(align='right') %} + {{ big_number(item['sending'], smaller=True) }} + {% endcall %} + {% call field(align='right') %} + {{ big_number(item['delivered'], smaller=True) }} + {% endcall %} + {% call field(align='right', status='error' if 0 else '') %} + {{ big_number(item['failed'], smaller=True) }} + {% endcall %} + {% endcall %} +{% endmacro %} + + {% block page_title %} Platform admin – GOV.UK Notify {% endblock %} @@ -45,46 +90,7 @@ - {% call(item, row_number) list_table( - service_stats, - caption="All services", - caption_visible=False, - field_headings=[ - 'Service', - hidden_field_heading('Status'), - right_aligned_field_heading('Sending'), - right_aligned_field_heading('Delivered'), - right_aligned_field_heading('Failed') - ], - field_headings_visible=True - ) %} - {% call field() %} -
- {{ item['name'] }} -
- {% endcall %} - {% if item['research_mode'] %} - {% call field() %} - research mode - {% endcall %} - {% elif not item['restricted'] %} - {% call field(status='error') %} - - Live - - {% endcall %} - {% else %} - {{ text_field('') }} - {% endif %} - {% call field(align='right') %} - {{ big_number(item['sending'], smaller=True) }} - {% endcall %} - {% call field(align='right') %} - {{ big_number(item['delivered'], smaller=True) }} - {% endcall %} - {% call field(align='right', status='error' if 0 else '') %} - {{ big_number(item['failed'], smaller=True) }} - {% endcall %} - {% endcall %} + + {{ services_table(service_stats, 'All services') }} {% endblock %} From 88fff50ea338947760cb97d93cb93c28907aa9c3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Sep 2016 11:38:16 +0100 Subject: [PATCH 3/4] Split platform admin page into live and trial mode It sucks having to scroll down the massive list of services just to see which ones are live. --- app/main/views/platform_admin.py | 7 ++++++- app/templates/views/platform-admin.html | 6 +++--- tests/app/main/views/test_platform_admin.py | 16 +++++++++------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index ca753782a..2ca78641a 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -25,7 +25,12 @@ def platform_admin(): def get_statistics(services): return { 'global_stats': create_global_stats(services), - 'service_stats': format_stats_by_service(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'] + ]), } diff --git a/app/templates/views/platform-admin.html b/app/templates/views/platform-admin.html index 519ecb4ec..38902bc7d 100644 --- a/app/templates/views/platform-admin.html +++ b/app/templates/views/platform-admin.html @@ -8,7 +8,7 @@ {% call(item, row_number) list_table( services, caption=caption, - caption_visible=False, + caption_visible=True, field_headings=[ 'Service', hidden_field_heading('Status'), @@ -89,8 +89,8 @@ - + {{ services_table(live_services, 'Live services') }} - {{ services_table(service_stats, 'All 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 a6b92979e..ca95d704a 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -30,14 +30,15 @@ def test_should_403_if_not_platform_admin(app_, active_user_with_permissions, mo assert response.status_code == 403 -@pytest.mark.parametrize('restricted, research_mode, displayed', [ - (True, False, ''), - (False, False, 'Live'), - (False, True, 'research mode'), - (True, True, 'research mode') +@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') ]) def test_should_show_research_and_restricted_mode( restricted, + table_index, research_mode, displayed, app_, @@ -60,7 +61,7 @@ def test_should_show_research_and_restricted_mode( mock_get_detailed_services.assert_called_once_with({'detailed': True}) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') # get second column, which contains flags as text. - assert page.tbody.select('td:nth-of-type(2)')[0].text.strip() == displayed + assert page.find_all('tbody')[table_index].find_all('td')[1].text.strip() == displayed def test_should_render_platform_admin_page( @@ -79,7 +80,8 @@ def test_should_render_platform_admin_page( resp_data = response.get_data(as_text=True) assert 'Platform admin' in resp_data assert 'Today' in resp_data - assert 'Services' 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 311ae4cfa48e284286fc24872ad3181b841c05ff Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Sep 2016 11:40:21 +0100 Subject: [PATCH 4/4] Sort services by newest created first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Other than which services are live it’s also interesting to know what services are getting created on Notify. So let’s put the newest ones at the top of the page. --- app/main/views/platform_admin.py | 9 +++++++-- tests/__init__.py | 3 ++- tests/conftest.py | 3 ++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 2ca78641a..7c883d7e9 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -18,7 +18,11 @@ def platform_admin(): services = service_api_client.get_services({'detailed': True})['data'] return render_template( 'views/platform-admin.html', - **get_statistics(services) + **get_statistics(sorted( + services, + key=lambda service: service['created_at'], + reverse=True + )) ) @@ -67,5 +71,6 @@ def format_stats_by_service(services): 'delivered': sum(stat['delivered'] for stat in stats), 'failed': sum(stat['failed'] for stat in stats), 'restricted': service['restricted'], - 'research_mode': service['research_mode'] + 'research_mode': service['research_mode'], + 'created_at': service['created_at'] } diff --git a/tests/__init__.py b/tests/__init__.py index d91d80cd0..03fa90167 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -65,7 +65,8 @@ def service_json( 'sms_sender': sms_sender, 'research_mode': research_mode, 'organisation': organisation, - 'branding': branding + 'branding': branding, + 'created_at': str(datetime.utcnow()) } diff --git a/tests/conftest.py b/tests/conftest.py index c884da70e..c7ddc0862 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -84,7 +84,8 @@ def mock_get_detailed_service(mocker, api_user_active): 'statistics': { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, 'sms': {'requested': 0, 'delivered': 0, 'failed': 0} - } + }, + 'created_at': str(datetime.utcnow()) } }