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/3] 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/3] 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/3] 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(