From 86abb7dba9db97c25ea6871a75e5a71fc4ec8626 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 9 Nov 2017 10:56:54 +0000 Subject: [PATCH 1/6] Refactor for number reuse --- app/main/views/dashboard.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index ce63fdc00..0db9331b7 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -10,7 +10,7 @@ from flask import ( Response, ) from flask_login import login_required - +from math import pow from notifications_utils.recipients import format_phone_number_human_readable from app.main import main @@ -244,7 +244,10 @@ def get_dashboard_partials(service_id): for job in job_api_client.get_jobs(service_id, limit_days=7, statuses=statuses_to_display)['data'] ] service = service_api_client.get_detailed_service(service_id) - column_width = 'column-third' if 'letter' in current_service['permissions'] else 'column-half' + number_of_columns = 3 if 'letter' in current_service['permissions'] else 2 + column_width = 'column-{}'.format( + {2: 'half', 3: 'third'}.get(number_of_columns) + ) return { 'upcoming': render_template( From 14129333563d649be1aface9984de4ab74609e8c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 9 Nov 2017 17:17:50 +0000 Subject: [PATCH 2/6] Make dashboard totals smaller if numbers are big Numbers over a billion overflow the two column layout. Numbers over one hundred thousand overflow the three column layout. This commit makes the type size smaller in these cases, so that the numbers still fit in the boxes. --- .../stylesheets/components/big-number.scss | 3 +- app/main/views/dashboard.py | 27 +++++-- app/templates/views/dashboard/_totals.html | 9 ++- tests/app/main/views/test_dashboard.py | 71 +++++++++++++++++++ 4 files changed, 100 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/components/big-number.scss b/app/assets/stylesheets/components/big-number.scss index 3afc5320a..57590d461 100644 --- a/app/assets/stylesheets/components/big-number.scss +++ b/app/assets/stylesheets/components/big-number.scss @@ -85,7 +85,8 @@ outline: 3px solid $yellow; } - .big-number { + .big-number, + .big-number-smaller { background: transparent; } diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 0db9331b7..fe6ac4fa2 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -10,7 +10,6 @@ from flask import ( Response, ) from flask_login import login_required -from math import pow from notifications_utils.recipients import format_phone_number_human_readable from app.main import main @@ -244,9 +243,15 @@ def get_dashboard_partials(service_id): for job in job_api_client.get_jobs(service_id, limit_days=7, statuses=statuses_to_display)['data'] ] service = service_api_client.get_detailed_service(service_id) - number_of_columns = 3 if 'letter' in current_service['permissions'] else 2 - column_width = 'column-{}'.format( - {2: 'half', 3: 'third'}.get(number_of_columns) + column_width, max_notifiction_count = get_column_properties( + 3 if 'letter' in current_service['permissions'] else 2 + ) + dashboard_totals = get_dashboard_totals(service['data']['statistics']), + highest_notification_count = max( + sum( + value[key] for key in {'requested', 'failed', 'delivered'} + ) + for key, value in dashboard_totals[0].items() ) return { @@ -264,8 +269,11 @@ def get_dashboard_partials(service_id): 'totals': render_template( 'views/dashboard/_totals.html', service_id=service_id, - statistics=get_dashboard_totals(service['data']['statistics']), - column_width=column_width + statistics=dashboard_totals[0], + column_width=column_width, + smaller_font_size=( + highest_notification_count > max_notifiction_count + ), ), 'template-statistics': render_template( 'views/dashboard/template-statistics.html', @@ -440,3 +448,10 @@ def get_tuples_of_financial_years( ) for year in range(start, end + 1) ) + + +def get_column_properties(number_of_columns): + return { + 2: ('column-half', 999999999), + 3: ('column-third', 99999), + }.get(number_of_columns) diff --git a/app/templates/views/dashboard/_totals.html b/app/templates/views/dashboard/_totals.html index 469a4b266..931fb3ad9 100644 --- a/app/templates/views/dashboard/_totals.html +++ b/app/templates/views/dashboard/_totals.html @@ -11,7 +11,8 @@ statistics['email']['failed_percentage'], statistics['email']['show_warning'], failure_link=url_for(".view_notifications", service_id=service_id, message_type='email', status='failed'), - link=url_for(".view_notifications", service_id=service_id, message_type='email', status='sending,delivered,failed') + link=url_for(".view_notifications", service_id=service_id, message_type='email', status='sending,delivered,failed'), + smaller=smaller_font_size ) }}
@@ -22,7 +23,8 @@ statistics['sms']['failed_percentage'], statistics['sms']['show_warning'], failure_link=url_for(".view_notifications", service_id=service_id, message_type='sms', status='failed'), - link=url_for(".view_notifications", service_id=service_id, message_type='sms', status='sending,delivered,failed') + link=url_for(".view_notifications", service_id=service_id, message_type='sms', status='sending,delivered,failed'), + smaller=smaller_font_size ) }}
{% if 'letter' in current_service['permissions'] %} @@ -34,7 +36,8 @@ statistics['letter']['failed_percentage'], statistics['letter']['show_warning'], failure_link=url_for(".view_notifications", service_id=service_id, message_type='letter', status='failed'), - link=url_for(".view_notifications", service_id=service_id, message_type='letter', status='') + link=url_for(".view_notifications", service_id=service_id, message_type='letter', status=''), + smaller=smaller_font_size ) }} {% endif %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 35a68dea4..61d490e09 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -461,6 +461,77 @@ def test_correct_columns_display_on_dashboard( assert len(page.select(column_name)) == expected_column_count +@pytest.mark.parametrize('permissions, totals, big_number_class, expected_column_count', [ + ( + ['email', 'sms'], + { + 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'sms': {'requested': 999999999, 'delivered': 0, 'failed': 0} + }, + '.big-number', + 2, + ), + ( + ['email', 'sms'], + { + 'email': {'requested': 1000000000, 'delivered': 0, 'failed': 0}, + 'sms': {'requested': 1000000, 'delivered': 0, 'failed': 0} + }, + '.big-number-smaller', + 2, + ), + ( + ['email', 'sms', 'letter'], + { + 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'sms': {'requested': 99999, 'delivered': 0, 'failed': 0}, + 'letter': {'requested': 99999, 'delivered': 0, 'failed': 0} + }, + '.big-number', + 3, + ), + ( + ['email', 'sms', 'letter'], + { + 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, + 'letter': {'requested': 100000, 'delivered': 0, 'failed': 0}, + }, + '.big-number-smaller', + 3, + ), +]) +def test_correct_font_size_for_big_numbers( + client_request, + mocker, + mock_get_service_templates, + mock_get_template_statistics, + mock_get_detailed_service, + mock_get_jobs, + service_one, + permissions, + totals, + big_number_class, + expected_column_count, +): + + service_one['permissions'] = permissions + + mocker.patch( + 'app.main.views.dashboard.get_dashboard_totals', + return_value=totals + ) + + page = client_request.get( + 'main.service_dashboard', + service_id=service_one['id'], + ) + + assert expected_column_count == len( + page.select('.big-number-with-status {}'.format(big_number_class)) + ) + + @freeze_time("2016-01-01 11:09:00.061258") def test_should_show_recent_jobs_on_dashboard( logged_in_client, From d6fec9f921cfbe1c1165db5555af040f2c0bb601 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 15 Nov 2017 09:59:52 +0000 Subject: [PATCH 3/6] Add named argument for clarity --- app/main/views/dashboard.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index fe6ac4fa2..52fe1bc57 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -244,7 +244,9 @@ def get_dashboard_partials(service_id): ] service = service_api_client.get_detailed_service(service_id) column_width, max_notifiction_count = get_column_properties( - 3 if 'letter' in current_service['permissions'] else 2 + number_of_columns=( + 3 if 'letter' in current_service['permissions'] else 2 + ) ) dashboard_totals = get_dashboard_totals(service['data']['statistics']), highest_notification_count = max( From d03df16db5660041b76f6ebaacd04597198ed431 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 16 Nov 2017 16:01:03 +0000 Subject: [PATCH 4/6] Added new template usage page which will replace template-activity The current template-activity page is slow as it is using the end point which uses notification_history and hence is timing out. This adds a new pages (so that they can be compared side by side) which will be hidden until is is approved with the larger data set and tested. --- app/main/views/dashboard.py | 55 +++++++++++++++++++ .../template_statistics_api_client.py | 6 ++ 2 files changed, 61 insertions(+) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 22a85e486..a1364d2cc 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -106,6 +106,61 @@ def template_history(service_id): ) +@main.route("/services//template-usage") +@login_required +@user_has_permissions('view_activity', admin_override=True) +def template_usage(service_id): + + year, current_financial_year = requested_and_current_financial_year(request) + stats = template_statistics_client.get_monthly_template_usage_for_service(service_id, year) + + stats = sorted(stats, key=lambda x: (x['count']), reverse=True) + + stats_by_month = list() + + for month in get_months_for_financial_year(year, time_format='%Y-%m'): + long_month = yyyy_mm_to_datetime(month).strftime('%B') + + template_used = list() + + for stat in stats: + if long_month == datetime(1900, stat['month'], 1).strftime("%B"): + template_used.append( + { + 'name': stat['name'], + 'type': stat['type'], + 'requested_count': stat['count'] + } + ) + + stats_by_month.append( + { + 'name': long_month, + 'templates_used': template_used + } + ) + + months = stats_by_month + + return render_template( + 'views/dashboard/all-template-statistics.html', + months=months, + stats=stats, + most_used_template_count=max( + max(( + template['requested_count'] + for template in month['templates_used'] + ), default=0) + for month in months + ), + years=get_tuples_of_financial_years( + partial(url_for, '.template_history', service_id=service_id), + end=current_financial_year, + ), + selected_year=year + ) + + @main.route("/services//usage") @login_required @user_has_permissions('manage_settings', admin_override=True) diff --git a/app/notify_client/template_statistics_api_client.py b/app/notify_client/template_statistics_api_client.py index 589cd5067..30f311f7f 100644 --- a/app/notify_client/template_statistics_api_client.py +++ b/app/notify_client/template_statistics_api_client.py @@ -26,6 +26,12 @@ class TemplateStatisticsApiClient(NotifyAdminAPIClient): url='/service/{}/notifications/templates/monthly?year={}'.format(service_id, year) )['data'] + def get_monthly_template_usage_for_service(self, service_id, year): + + return self.get( + url='/service/{}/notifications/templates_usage/monthly?year={}'.format(service_id, year) + )['stats'] + def get_template_statistics_for_template(self, service_id, template_id): return self.get( From de9548bba228e4791e4df32d316d8bf8828f2a04 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Fri, 17 Nov 2017 09:26:03 +0000 Subject: [PATCH 5/6] Only Allow Admins Changed the decorator to only allow admin users access to the page. --- app/main/views/dashboard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index a1364d2cc..675f5e5e0 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -108,7 +108,7 @@ def template_history(service_id): @main.route("/services//template-usage") @login_required -@user_has_permissions('view_activity', admin_override=True) +@user_has_permissions(admin_override=True) def template_usage(service_id): year, current_financial_year = requested_and_current_financial_year(request) From a8b854425a64686056673ee16d8627335aa0c1eb Mon Sep 17 00:00:00 2001 From: Pete Herlihy Date: Fri, 17 Nov 2017 10:39:38 +0000 Subject: [PATCH 6/6] 104 to 109 and 45-46 orgs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Department for Education – Find a Career Gravesham Borough Council – Gravesham BC Crown Commercial Service – CCS Power BI Department for International Trade – Export Support Alpha Crown Commercial Service – GCloud Assurance --- app/templates/views/signedout.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html index 5ef7221c6..1eb27e679 100644 --- a/app/templates/views/signedout.html +++ b/app/templates/views/signedout.html @@ -120,12 +120,12 @@
-
104
+
109
services
-
45
+
46
organisations