diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index b58d74b3f..570d997e0 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -210,7 +210,7 @@ details summary { table th, table td { - font-size: 19px; + @include core-19; } } diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index 1b65b6a4b..16fd35d07 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -75,3 +75,8 @@ } } + +.failure-highlight { + @include bold-19; + color: $error-colour; +} diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index f171faf26..8de2ee003 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -19,7 +19,14 @@ from app import ( template_statistics_client ) from app.statistics_utils import get_formatted_percentage, add_rate_to_job -from app.utils import user_has_permissions, get_current_financial_year +from app.utils import ( + user_has_permissions, + get_current_financial_year, + FAILURE_STATUSES, + SENDING_STATUSES, + DELIVERED_STATUSES, + REQUESTED_STATUSES, +) # This is a placeholder view method to be replaced @@ -79,11 +86,7 @@ def template_history(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def usage(service_id): - current_financial_year = get_current_financial_year() - try: - year = int(request.args.get('year', current_financial_year)) - except ValueError: - abort(404) + year, current_financial_year = requested_and_current_financial_year(request) return render_template( 'views/usage.html', months=list(get_free_paid_breakdown_for_billable_units( @@ -102,15 +105,25 @@ def usage(service_id): ) -@main.route("/services//weekly") +@main.route("/services//monthly") @login_required @user_has_permissions('manage_settings', admin_override=True) -def weekly(service_id): - stats = service_api_client.get_weekly_notification_stats(service_id)['data'] +def monthly(service_id): + year, current_financial_year = requested_and_current_financial_year(request) return render_template( - 'views/weekly.html', - days=format_weekly_stats_to_list(stats), - now=datetime.utcnow() + 'views/dashboard/monthly.html', + months=format_monthly_stats_to_list( + service_api_client.get_monthly_notification_stats(service_id, year)['data'] + ), + years=[ + ( + 'financial year', + year, + url_for('.monthly', service_id=service_id, year=year), + '{} to {}'.format(year, year + 1), + ) for year in range(2015, current_financial_year + 1) + ], + selected_year=year, ) @@ -198,22 +211,32 @@ def calculate_usage(usage): } -def format_weekly_stats_to_list(historical_stats): - out = [] - for week, weekly_stats in historical_stats.items(): - for stats in weekly_stats.values(): - stats['failure_rate'] = get_formatted_percentage(stats['failed'], stats['requested']) +def format_monthly_stats_to_list(historical_stats): + return sorted(( + dict( + date=key, + future=YYYY_MM_to_datetime(key) > datetime.utcnow(), + name=YYYY_MM_to_datetime(key).strftime('%B'), + **aggregate_status_types(value) + ) for key, value in historical_stats.items() + ), key=lambda x: x['date']) - week_start = dateutil.parser.parse(week) - week_end = week_start + timedelta(days=6) - weekly_stats.update({ - 'week_start': week, - 'week_end': week_end.date().isoformat(), - 'week_end_datetime': week_end, - }) - out.append(weekly_stats) - return sorted(out, key=lambda x: x['week_start'], reverse=True) +def YYYY_MM_to_datetime(string): + return datetime(int(string[0:4]), int(string[5:7]), 1) + + +def aggregate_status_types(counts_dict): + return get_dashboard_totals({ + '{}_counts'.format(message_type): { + 'failed': sum( + stats.get(status, 0) for status in FAILURE_STATUSES + ), + 'requested': sum( + stats.get(status, 0) for status in REQUESTED_STATUSES + ) + } for message_type, stats in counts_dict.items() + }) def get_months_for_financial_year(year): @@ -269,3 +292,13 @@ def get_free_paid_breakdown_for_month( 'paid': monthly_usage, 'free': 0 } + + +def requested_and_current_financial_year(request): + try: + return ( + int(request.args.get('year', get_current_financial_year())), + get_current_financial_year(), + ) + except ValueError: + abort(404) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 2c282a7d3..846b23074 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -33,6 +33,9 @@ from app.utils import ( generate_notifications_csv, get_help_argument, get_template, + REQUESTED_STATUSES, + FAILURE_STATUSES, + SENDING_STATUSES, ) from app.statistics_utils import add_rate_to_job @@ -53,12 +56,10 @@ def _parse_filter_args(filter_dict): def _set_status_filters(filter_args): status_filters = filter_args.get('status', []) - all_failure_statuses = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] - all_sending_statuses = ['created', 'sending'] return list(OrderedSet(chain( - (status_filters or all_sending_statuses + ['delivered'] + all_failure_statuses), - all_sending_statuses if 'sending' in status_filters else [], - all_failure_statuses if 'failed' in status_filters else [] + (status_filters or REQUESTED_STATUSES), + SENDING_STATUSES if 'sending' in status_filters else [], + FAILURE_STATUSES if 'failed' in status_filters else [] ))) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 211eb46a1..8ab6049ae 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -219,8 +219,8 @@ class ServiceAPIClient(NotifyAdminAPIClient): params=dict(year=year) ) - def get_weekly_notification_stats(self, service_id): - return self.get(url='/service/{}/notifications/weekly'.format(service_id)) + def get_monthly_notification_stats(self, service_id, year): + return self.get(url='/service/{}/notifications/monthly?year={}'.format(service_id, year)) def get_whitelist(self, service_id): return self.get(url='/service/{}/whitelist'.format(service_id)) diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html index 9ea92fbed..153030719 100644 --- a/app/templates/views/dashboard/dashboard.html +++ b/app/templates/views/dashboard/dashboard.html @@ -31,8 +31,8 @@ {{ ajax_block(partials, updates_url, 'totals') }} {{ show_more( - url_for('.weekly', service_id=current_service.id), - 'Compare to previous weeks' + url_for('.monthly', service_id=current_service.id), + 'See activity breakdown' ) }} {% if partials['has_template_statistics'] %} diff --git a/app/templates/views/dashboard/monthly.html b/app/templates/views/dashboard/monthly.html new file mode 100644 index 000000000..dd4e13fbc --- /dev/null +++ b/app/templates/views/dashboard/monthly.html @@ -0,0 +1,71 @@ +{% from "components/big-number.html" import big_number_with_status, big_number %} +{% from "components/pill.html" import pill %} +{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading, row_heading %} +{% from "components/message-count-label.html" import message_count_label %} + +{% extends "withnav_template.html" %} + +{% block page_title %} + Previous weeks – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

+ Activity breakdown +

+
+ {{ pill( + 'financial year', + items=years, + current_value=selected_year, + big_number_args={'smallest': True}, + ) }} +
+ {% if months %} +
+ {% call(month, row_index) list_table( + months, + caption="Total spend", + caption_visible=False, + empty_message='', + field_headings=[ + 'Month', + 'Emails', + 'Text messages', + ], + field_headings_visible=False + ) %} + {% if not month.future %} + {% call row_heading() %} + {{ month.name }} + {% endcall %} + {% for counts, template_type in [ + (month.email_counts, 'email'), + (month.sms_counts, 'sms') + ] %} + {% call field(align='left') %} + {{ big_number( + counts.requested, + message_count_label(counts.requested, template_type, suffix=''), + smallest=True, + ) }} + {% if counts.requested %} + + {{ counts.failed }} failed + + {% else %} + – + {% endif %} + {% endcall %} + {% endfor %} + {% endif %} + {% endcall %} +
+ {% endif %} + +

+ Financial year ends 31 March. +

+ +{% endblock %} diff --git a/app/templates/views/weekly.html b/app/templates/views/weekly.html deleted file mode 100644 index 5a5a87493..000000000 --- a/app/templates/views/weekly.html +++ /dev/null @@ -1,50 +0,0 @@ -{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading %} - -{% extends "withnav_template.html" %} - -{% block page_title %} - Previous weeks – 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.email.requested }} - {% endcall %} - {% call field(align='right') %} - {{ item.email.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/app/utils.py b/app/utils.py index 002a2dd01..0c03a86c5 100644 --- a/app/utils.py +++ b/app/utils.py @@ -32,6 +32,12 @@ import pyexcel.ext.xlsx import pyexcel.ext.ods3 +SENDING_STATUSES = ['created', 'pending', 'sending'] +DELIVERED_STATUSES = ['delivered'] +FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] +REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES + + class BrowsableItem(object): """ Maps for the template browse-list. diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index e3bb754c0..594e79d93 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -8,8 +8,9 @@ from freezegun import freeze_time from app.main.views.dashboard import ( get_dashboard_totals, - format_weekly_stats_to_list, - get_free_paid_breakdown_for_billable_units + format_monthly_stats_to_list, + get_free_paid_breakdown_for_billable_units, + aggregate_status_types, ) from tests import validate_route_permission @@ -534,49 +535,67 @@ def test_get_dashboard_totals_adds_warning(failures, expected): assert get_dashboard_totals(stats)['sms']['show_warning'] == expected -def test_format_weekly_stats_to_list_empty_case(): - assert format_weekly_stats_to_list({}) == [] +def test_format_monthly_stats_empty_case(): + assert format_monthly_stats_to_list({}) == [] -def test_format_weekly_stats_to_list_sorts_by_week(): - stats = { - '2016-07-04': {}, - '2016-07-11': {}, - '2016-07-18': {}, - '2016-07-25': {} +def test_format_monthly_stats_labels_month(): + resp = format_monthly_stats_to_list({'2016-07': {}}) + assert resp[0]['name'] == 'July' + + +def test_format_monthly_stats_has_stats_with_failure_rate(): + resp = format_monthly_stats_to_list({ + '2016-07': {'sms': _stats(3, 1, 2)} + }) + assert resp[0]['sms_counts'] == { + 'failed': 2, + 'failed_percentage': '66.7', + 'requested': 3, + 'show_warning': True, } - resp = format_weekly_stats_to_list(stats) - assert resp[0]['week_start'] == '2016-07-25' - assert resp[1]['week_start'] == '2016-07-18' - assert resp[2]['week_start'] == '2016-07-11' - assert resp[3]['week_start'] == '2016-07-04' -def test_format_weekly_stats_to_list_includes_datetime_for_comparison(): - stats = { - '2016-07-25': {} - } - resp = format_weekly_stats_to_list(stats) - assert resp == [{ - 'week_start': '2016-07-25', - 'week_end': '2016-07-31', - 'week_end_datetime': datetime(2016, 7, 31, 0, 0, 0) - }] - - -def test_format_weekly_stats_to_list_has_stats_with_failure_rate(): - stats = { - '2016-07-25': {'sms': _stats(3, 1, 2)} - } - resp = format_weekly_stats_to_list(stats) - assert resp[0]['sms']['failure_rate'] == '66.7' - assert resp[0]['sms']['requested'] == 3 +def test_format_monthly_stats_works_for_email_letter(): + resp = format_monthly_stats_to_list({ + '2016-07': { + 'sms': {}, + 'email': {}, + 'letter': {}, + } + }) + assert isinstance(resp[0]['sms_counts'], dict) + assert isinstance(resp[0]['email_counts'], dict) + assert isinstance(resp[0]['letter_counts'], dict) def _stats(requested, delivered, failed): return {'requested': requested, 'delivered': delivered, 'failed': failed} +@pytest.mark.parametrize('dict_in, expected_failed, expected_requested', [ + ( + {}, + 0, + 0 + ), + ( + {'temporary-failure': 1, 'permanent-failure': 1, 'technical-failure': 1}, + 3, + 3, + ), + ( + {'created': 1, 'pending': 1, 'sending': 1, 'delivered': 1}, + 0, + 4, + ), +]) +def test_aggregate_status_types(dict_in, expected_failed, expected_requested): + sms_counts = aggregate_status_types({'sms': dict_in})['sms_counts'] + assert sms_counts['failed'] == expected_failed + assert sms_counts['requested'] == expected_requested + + @pytest.mark.parametrize( 'now, expected_number_of_months', [ (freeze_time("2017-12-31 11:09:00.061258"), 12), diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index ebe0002b4..1f7d58da0 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -68,11 +68,15 @@ def test_get_jobs_shows_page_links( "status_argument, expected_api_call", [ ( '', - ['created', 'sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] + [ + 'created', 'pending', 'sending', + 'delivered', + 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', + ] ), ( 'sending', - ['sending', 'created'] + ['sending', 'created', 'pending'] ), ( 'delivered', @@ -304,11 +308,15 @@ def test_should_show_updates_for_one_job_as_json( "status_argument, expected_api_call", [ ( '', - ['created', 'sending', 'delivered', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] + [ + 'created', 'pending', 'sending', + 'delivered', + 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', + ] ), ( 'sending', - ['sending', 'created'] + ['sending', 'created', 'pending'] ), ( 'delivered',