From 50427ecd3fc3fca539bb00f04eaa0b9926d6ed79 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 3 May 2016 13:25:22 +0100 Subject: [PATCH] Add a page to show delivery rates week-by-week Implements https://github.com/alphagov/notifications-api/pull/286 Will always show weeks as Monday to Sunday. --- app/__init__.py | 6 ++ .../stylesheets/components/big-number.scss | 15 +++++ app/assets/stylesheets/components/table.scss | 6 +- app/assets/stylesheets/views/dashboard.scss | 4 +- app/main/views/dashboard.py | 61 ++++++++++++++----- app/notify_client/statistics_api_client.py | 11 ++++ app/templates/components/big-number.html | 7 ++- app/templates/views/dashboard/dashboard.html | 3 + .../views/dashboard/template-statistics.html | 14 +++-- app/templates/views/dashboard/today.html | 8 +-- app/templates/views/weekly.html | 50 +++++++++++++++ tests/app/main/views/test_dashboard.py | 11 ++-- tests/conftest.py | 9 +++ 13 files changed, 170 insertions(+), 35 deletions(-) create mode 100644 app/templates/views/weekly.html diff --git a/app/__init__.py b/app/__init__.py index c89348cea..f8d86f8c9 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -101,6 +101,7 @@ def create_app(): application.add_template_filter(valid_phone_number) application.add_template_filter(linkable_name) application.add_template_filter(format_date) + application.add_template_filter(format_date_short) application.after_request(useful_headers_after_request) application.after_request(save_service_after_request) @@ -201,6 +202,11 @@ def format_date(date): return date.strftime('%A %d %B %Y') +def format_date_short(date): + date = dateutil.parser.parse(date) + return date.strftime('%d %B').lstrip('0') + + def valid_phone_number(phone_number): try: validate_phone_number(phone_number) diff --git a/app/assets/stylesheets/components/big-number.scss b/app/assets/stylesheets/components/big-number.scss index f4ef4cac6..a8372ac18 100644 --- a/app/assets/stylesheets/components/big-number.scss +++ b/app/assets/stylesheets/components/big-number.scss @@ -24,6 +24,21 @@ color: $white; position: relative; + &-show-more-link { + @include core-16; + display: block; + padding: 0.75em 0 0.5625em 0; + margin-bottom: $gutter-half; + text-align: center; + border-bottom: 1px solid $border-colour; + + &:focus { + outline: none; + color: $text-colour; + border-bottom: 1px solid $brown; + } + } + .big-number { padding: 15px; position: relative; diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 32b3e7512..efe70d644 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -4,7 +4,7 @@ .table-heading { text-align: left; - margin: 40px 0 5px 0; + margin: 40px 0 $gutter-half 0; } .table-field-headings { @@ -111,10 +111,10 @@ .table-show-more-link { @include core-16; color: $secondary-text-colour; - margin-top: -20px; + margin-top: -30px; margin-bottom: $gutter * 1.3333; border-bottom: 1px solid $border-colour; - padding-bottom: 10px; + padding: 0.75em 0 0.5625em 0; text-align: center; } diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index b79e6db4e..d4dfb1159 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -1,6 +1,6 @@ .dashboard { table th { - @include bold-19; + font-size: 0; } } @@ -13,7 +13,7 @@ @include core-16; border-top: 1px solid $border-colour; border-bottom: 1px solid $border-colour; - padding: 10px 0; + padding: 0.75em 0 0.5625em 0; text-align: center; } diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 2bf54a49f..75df0003c 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -1,4 +1,5 @@ -from datetime import date +from datetime import datetime, date, timedelta +from dateutil import parser from collections import namedtuple from itertools import groupby from functools import reduce @@ -90,40 +91,72 @@ def usage(service_id): ) -def add_rates_to(delivery_statistics): +@main.route("/services//weekly") +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def weekly(service_id): - keys = [ + earliest_date = date(2016, 4, 1) # start of tax year + while earliest_date.weekday() != 0: # 0 for monday + earliest_date -= timedelta(days=1) + + return render_template( + 'views/weekly.html', + days=( + add_rates_to(day) for day in + statistics_api_client.get_7_day_aggregate_for_service( + service_id, + date_from=earliest_date + )['data'] + ), + now=datetime.now() + ) + + +def sum_of_statistics(delivery_statistics): + + statistics_keys = ( 'emails_delivered', 'emails_requested', 'emails_failed', 'sms_requested', 'sms_delivered', 'sms_failed' - ] + ) if not delivery_statistics or not delivery_statistics[0]: return { - key: 0 for key in keys + key: 0 for key in statistics_keys } - sum_of_statistics = reduce( + return reduce( lambda x, y: { key: x.get(key, 0) + y.get(key, 0) - for key in keys + for key in statistics_keys }, delivery_statistics ) + +def add_rates_to(delivery_statistics): + return dict( emails_failure_rate=( - "{0:.1f}".format((float(sum_of_statistics['emails_failed']) / sum_of_statistics['emails_requested'] * 100)) - if sum_of_statistics['emails_requested'] else 0 + "{0:.1f}".format( + float(delivery_statistics['emails_failed']) / delivery_statistics['emails_requested'] * 100 + ) + if delivery_statistics['emails_requested'] else 0 ), sms_failure_rate=( - "{0:.1f}".format((float(sum_of_statistics['sms_failed']) / sum_of_statistics['sms_requested'] * 100)) - if sum_of_statistics['sms_requested'] else 0 + "{0:.1f}".format( + float(delivery_statistics['sms_failed']) / delivery_statistics['sms_requested'] * 100 + ) + if delivery_statistics['sms_requested'] else 0 ), - **sum_of_statistics + week_end_datetime=parser.parse( + delivery_statistics.get('week_end', str(datetime.now())) + ), + **delivery_statistics ) @@ -167,9 +200,9 @@ def get_dashboard_statistics_for_service(service_id): emails_sent = usage['data'].get('email_count', 0) return { - 'statistics': add_rates_to( + 'statistics': add_rates_to(sum_of_statistics( statistics_api_client.get_statistics_for_service(service_id, limit_days=7)['data'] - ), + )), 'template_statistics': aggregate_usage( template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7) ), diff --git a/app/notify_client/statistics_api_client.py b/app/notify_client/statistics_api_client.py index 3492d7e91..966a4ffc7 100644 --- a/app/notify_client/statistics_api_client.py +++ b/app/notify_client/statistics_api_client.py @@ -20,3 +20,14 @@ class StatisticsApiClient(BaseAPIClient): url='/service/{}/notifications-statistics'.format(service_id), params=params ) + + def get_7_day_aggregate_for_service(self, service_id, date_from=None, week_count=None): + params = {} + if date_from is not None: + params['date_from'] = date_from + if week_count is not None: + params['week_count'] = week_count + return self.get( + url='/service/{}/notifications-statistics/seven_day_aggregate'.format(service_id), + params=params + ) diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index dbc12bd40..6a5c9cbe7 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -26,7 +26,9 @@ failure_percentage, danger_zone=False, failure_link=None, - label_link=None + label_link=None, + show_more_link=None, + show_more_text='' ) %}
{{ big_number(number, label, label_link) }} @@ -44,4 +46,7 @@ {% endif %}
+ {% if show_more_link and show_more_text %} + {{ show_more_text }} + {% endif %} {% endmacro %} diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html index e1d43b16c..68e42ae45 100644 --- a/app/templates/views/dashboard/dashboard.html +++ b/app/templates/views/dashboard/dashboard.html @@ -20,6 +20,9 @@ {% include 'views/dashboard/no-permissions-banner.html' %} {% endif %} +

+ In the last 7 days +

{% include 'views/dashboard/today.html' %} diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index a29651ee2..ee69efdd4 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -1,21 +1,23 @@ {% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading %} + {% call(item, row_number) list_table( template_statistics, - caption="By template", - caption_visible=False, + caption="Templates", + caption_visible=True, empty_message='You haven’t used any templates {}'.format(period), - field_headings=['Template', hidden_field_heading('Type'), right_aligned_field_heading('Messages sent')] + field_headings=['Template', hidden_field_heading('Type'), hidden_field_heading('Messages sent')], + field_headings_visible=False ) %} {% call field() %} {{ item.template.name }} {% endcall %} - {% call field() %} - {{'Text message' if 'sms' == item.template.template_type else 'Email'}} - {% endcall %} {% call field(align='right') %} {{ item.usage_count }} {% endcall %} + {% call field() %} + {{'text messages sent' if 'sms' == item.template.template_type else 'emails sent'}} + {% endcall %} {% endcall %} diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index ad1ad74c1..16dcaa16e 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -7,10 +7,7 @@ data-interval-seconds="2" aria-live="polite" > -

- In the last 7 days -

-
+
{{ big_number_with_status( statistics.emails_requested, @@ -33,6 +30,9 @@ label_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='delivered,failed') ) }}
+
+ Compare to previous weeks +
{% with period = "in the last 7 days" %} {% include 'views/dashboard/template-statistics.html' %} diff --git a/app/templates/views/weekly.html b/app/templates/views/weekly.html new file mode 100644 index 000000000..10a516290 --- /dev/null +++ b/app/templates/views/weekly.html @@ -0,0 +1,50 @@ +{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading %} + +{% extends "withnav_template.html" %} + +{% block page_title %} + Daily – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

+ Previous weeks +

+ {% call(item, row_number) list_table( + days, + caption="Daily", + caption_visible=False, + empty_message='No data found', + field_headings=[ + hidden_field_heading('Day'), + right_aligned_field_heading('Emails'), + right_aligned_field_heading('Failure rate'), + right_aligned_field_heading('Text messages'), + right_aligned_field_heading('Failure rate') + ] + ) %} + {% call field() %} + {{ item.week_start|format_date_short }} to + {% if item.week_end_datetime > now %} + today + {% else %} + {{ item.week_end|format_date_short }} + {% endif %} + {% endcall %} + {% call field(align='right') %} + {{ item.emails_requested }} + {% endcall %} + {% call field(align='right') %} + {{ item.emails_failure_rate }}% + {% endcall %} + {% call field(align='right') %} + {{ item.sms_requested }} + {% endcall %} + {% call field(align='right') %} + {{ item.sms_failure_rate }}% + {% endcall %} + {% endcall %} + + +{% endblock %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 35a062ffb..a58a9bb0e 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -61,6 +61,7 @@ def test_should_show_recent_templates_on_dashboard(app_, mock_get_service, mock_get_service_templates, mock_get_service_statistics, + mock_get_aggregate_service_statistics, mock_get_user, mock_get_user_by_email, mock_login, @@ -82,7 +83,7 @@ def test_should_show_recent_templates_on_dashboard(app_, mock_template_stats.assert_called_once_with(SERVICE_ONE_ID, limit_days=7) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - headers = [header.text.strip() for header in page.find_all('h2')] + headers = [header.text.strip() for header in page.find_all('h2') + page.find_all('h1')] assert 'Test Service' in headers assert 'In the last 7 days' in headers template_usage_headers = [th.text.strip() for th in page.thead.find_all('th')] @@ -94,12 +95,12 @@ def test_should_show_recent_templates_on_dashboard(app_, first_row = page.tbody.find_all('tr')[0] table_data = first_row.find_all('td') assert len(table_data) == 3 - assert table_data[2].text.strip() == '206' + assert table_data[1].text.strip() == '206' second_row = page.tbody.find_all('tr')[1] table_data = second_row.find_all('td') assert len(table_data) == 3 - assert table_data[2].text.strip() == '13' + assert table_data[1].text.strip() == '13' def test_should_show_all_templates_on_template_statistics_page( @@ -137,12 +138,12 @@ def test_should_show_all_templates_on_template_statistics_page( first_row = page.tbody.find_all('tr')[0] table_data = first_row.find_all('td') assert len(table_data) == 3 - assert table_data[2].text.strip() == '206' + assert table_data[1].text.strip() == '206' second_row = page.tbody.find_all('tr')[1] table_data = second_row.find_all('td') assert len(table_data) == 3 - assert table_data[2].text.strip() == '13' + assert table_data[1].text.strip() == '13' def _test_dashboard_menu(mocker, app_, usr, service, permissions): diff --git a/tests/conftest.py b/tests/conftest.py index 89b01d747..20248674f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -191,6 +191,15 @@ def mock_get_service_statistics(mocker): 'app.statistics_api_client.get_statistics_for_service', side_effect=_create) +@pytest.fixture(scope='function') +def mock_get_aggregate_service_statistics(mocker): + def _create(service_id, limit_days=None): + return {'data': [{}]} + + return mocker.patch( + 'app.statistics_api_client.get_7_day_aggregate_for_service', side_effect=_create) + + @pytest.fixture(scope='function') def mock_get_service_template(mocker): def _get(service_id, template_id):