From 7e699c93ca9163b66318a51d1ceb5ce3b635a3f3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 25 Oct 2019 15:01:27 +0100 Subject: [PATCH 1/4] Always show letters on the usage page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even if your service doesn’t send letters now, it might have done previously. The original reason for hiding letters was because it wasn’t a mature feature. But now that it is, we should make it discoverable even for existing teams. So that means not conditionally hiding it. This is the same thing we do for emails/texts, where even if you switch them off they still show up on the dashboard and usage page. --- app/main/views/dashboard.py | 5 +- app/templates/views/usage.html | 117 ------------------------- tests/app/main/views/test_dashboard.py | 4 +- 3 files changed, 3 insertions(+), 123 deletions(-) delete mode 100644 app/templates/views/usage.html diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index b7e64997c..60f7a8e8b 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -133,11 +133,8 @@ def usage(service_id): units = billing_api_client.get_billable_units(service_id, year) yearly_usage = billing_api_client.get_service_usage(service_id, year) - usage_template = 'views/usage.html' - if current_service.has_permission('letter'): - usage_template = 'views/usage-with-letters.html' return render_template( - usage_template, + 'views/usage-with-letters.html', months=list(get_free_paid_breakdown_for_billable_units( year, free_sms_allowance, diff --git a/app/templates/views/usage.html b/app/templates/views/usage.html deleted file mode 100644 index fb10e8581..000000000 --- a/app/templates/views/usage.html +++ /dev/null @@ -1,117 +0,0 @@ -{% from "components/big-number.html" import big_number %} -{% from "components/message-count-label.html" import message_count_label %} -{% from "components/table.html" import list_table, field, hidden_field_heading, row_heading, text_field %} -{% from "components/pill.html" import pill %} - -{% extends "withnav_template.html" %} - -{% block service_page_title %} - Usage -{% endblock %} - -{% block maincolumn_content %} - -

Usage

- -
- {{ pill(years, selected_year, big_number_args={'smallest': True}) }} -
-
-
-
-

Emails

-
- {{ big_number(emails_sent, 'sent', smaller=True) }} - {{ big_number("Unlimited", 'free allowance', smaller=True) }} -
-
-
-

Text messages

-
- {{ big_number(sms_sent, 'sent', smaller=True) }} - {{ big_number(sms_free_allowance, 'free allowance', smaller=True) }} - {{ big_number(sms_allowance_remaining, 'free allowance remaining', smaller=True) }} - {% if sms_chargeable %} - {{ big_number( - sms_chargeable, - 'at {:.2f} pence per message'.format(sms_rate * 100), - smaller=True - ) }} - {% endif %} -
-
-
- -
-
-
-   -
-
-
-
- {{ big_number( - (sms_chargeable * sms_rate), - 'spent', - currency="£", - smaller=True - ) }} -
-
-
- - {% if months %} -
- {% call(month, row_index) list_table( - months, - caption="Total spend", - caption_visible=False, - empty_message='', - field_headings=[ - 'By month', - hidden_field_heading('Cost'), - ], - field_headings_visible=True - ) %} - {% call row_heading() %} - {{ month.name }} - {% endcall %} - {% call field(align='left') %} - {{ big_number( - sms_rate * month.paid, - currency="£", - smallest=True - ) }} -
    - {% if month.free %} -
  • {{ "{:,}".format(month.free) }} free {{ message_count_label(month.free, 'sms', '') }}
    • - {% endif %} - {% if month.paid %} -
  • {{ "{:,}".format(month.paid) }} {{ message_count_label(month.free, 'sms', '') }}at - {{- ' {:.2f}p'.format(sms_rate * 100) }}
    • - {% endif %} - {% if not (month.free or month.paid) %} - - {% endif %} -
- {% endcall %} - {% endcall %} -
- {% endif %} -
- -
-
-

- Financial year ends 31 March. -

-
-
-

- What counts as 1 text message?
- See pricing. -

-
-
- -{% endblock %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index c06057af1..932b9065e 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -795,7 +795,7 @@ def test_usage_page( mock_get_usage.assert_called_once_with(SERVICE_ONE_ID, 2011) mock_get_free_sms_fragment_limit.assert_called_with(SERVICE_ONE_ID, 2011) - cols = page.find_all('div', {'class': 'column-half'}) + cols = page.find_all('div', {'class': 'column-one-third'}) nav = page.find('ul', {'class': 'pill', 'role': 'tablist'}) nav_links = nav.find_all('a') @@ -813,7 +813,7 @@ def test_usage_page( assert 'April' in table assert 'February' in table assert 'March' in table - assert '£15.84' in table + assert '£20.59' in table assert '140 free text messages' in table assert '£20.30' in table assert '1,230 text messages at 1.65p' in table From 2732a0b80b0370e30f4968466ac0e42ba2fc6900 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 25 Oct 2019 15:07:34 +0100 Subject: [PATCH 2/4] Make things line up on the usage page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mixture of three column/two column layouts on this page has always looked a bit disjointed. And since the left column will only even contain the names of months, which are short, it doesn’t need a full half of the page width. --- app/assets/stylesheets/components/table.scss | 14 ++++++++++++++ app/templates/views/usage-with-letters.html | 8 ++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 840d535f6..7be4fb25f 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -98,6 +98,20 @@ } +.usage-table { + + .table-field-heading-first { + width: 35%; // 33.33% + fudge + } + + .table-row { + th { + width: 17.5% // 16.67% + fudge + } + } + +} + %table-field, .table-field { diff --git a/app/templates/views/usage-with-letters.html b/app/templates/views/usage-with-letters.html index 30cc88bd2..294d25dfd 100644 --- a/app/templates/views/usage-with-letters.html +++ b/app/templates/views/usage-with-letters.html @@ -78,7 +78,7 @@ {% if months %} -
+
{% call(month, row_index) list_table( months, caption="Total spend", @@ -124,12 +124,12 @@
-
+

- Financial year ends 31 March. + Financial year ends 31 March

-
+

What counts as 1 text message?
See pricing. From 6c841affc840cc4b3ae58843bcc807da58b6e1c8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 25 Oct 2019 15:34:01 +0100 Subject: [PATCH 3/4] Always show letter counts on dashboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We hid letters originally because it wasn’t a mature feature. We rolled it out by letting teams choose to use it (#1803) and then automatically giving it to new teams (notifications-api/#1600). This commit doesn’t change who has access to letters, but it does make it more discoverable by revealing it in the UI. This is the same thing we do for emails/texts, where even if you switch them off they still show up on the dashboard and usage page. --- app/main/views/dashboard.py | 7 +-- app/templates/views/dashboard/_totals.html | 26 +++++---- tests/app/main/views/test_dashboard.py | 62 ++++------------------ 3 files changed, 23 insertions(+), 72 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 60f7a8e8b..8fadf10c6 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -290,11 +290,8 @@ def get_dashboard_partials(service_id): ] stats = aggregate_notifications_stats(all_statistics) - column_width, max_notifiction_count = get_column_properties( - number_of_columns=( - 3 if current_service.has_permission('letter') else 2 - ) - ) + column_width, max_notifiction_count = get_column_properties(3) + dashboard_totals = get_dashboard_totals(stats), highest_notification_count = max( sum( diff --git a/app/templates/views/dashboard/_totals.html b/app/templates/views/dashboard/_totals.html index a7a9de599..778862a4c 100644 --- a/app/templates/views/dashboard/_totals.html +++ b/app/templates/views/dashboard/_totals.html @@ -27,19 +27,17 @@ smaller=smaller_font_size ) }}

- {% if current_service.has_permission('letter') %} -
- {{ big_number_with_status( - statistics['letter']['requested'], - message_count_label(statistics['letter']['requested'], 'letter', suffix='sent'), - statistics['letter']['failed'], - 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=''), - smaller=smaller_font_size - ) }} -
- {% endif %} +
+ {{ big_number_with_status( + statistics['letter']['requested'], + message_count_label(statistics['letter']['requested'], 'letter', suffix='sent'), + statistics['letter']['failed'], + 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=''), + smaller=smaller_font_size + ) }} +
diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 932b9065e..39d21d79e 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -645,71 +645,26 @@ def test_should_show_upcoming_jobs_on_dashboard( assert table_rows[1].find_all('td')[0].text.strip() == '1' -@pytest.mark.parametrize('permissions, column_name, expected_column_count', [ - (['email', 'sms'], '.column-half', 2), - (['email', 'letter'], '.column-third', 3), - (['email', 'sms', 'letter'], '.column-third', 3) -]) -def test_correct_columns_display_on_dashboard( - client_request, - mock_get_service_templates, - mock_get_template_statistics, - mock_get_service_statistics, - mock_get_jobs, - service_one, - permissions, - expected_column_count, - column_name -): - - service_one['permissions'] = permissions - - page = client_request.get( - 'main.service_dashboard', - service_id=service_one['id'] - ) - - assert len(page.select(column_name)) == expected_column_count - - -@pytest.mark.parametrize('permissions, totals, big_number_class, expected_column_count', [ +@pytest.mark.parametrize('permissions', ( + ['email', 'sms'], + ['email', 'sms', 'letter'], +)) +@pytest.mark.parametrize('totals, big_number_class', [ ( - ['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( @@ -723,7 +678,6 @@ def test_correct_font_size_for_big_numbers( permissions, totals, big_number_class, - expected_column_count, ): service_one['permissions'] = permissions @@ -738,9 +692,11 @@ def test_correct_font_size_for_big_numbers( service_id=service_one['id'], ) - assert expected_column_count == len( + assert len(page.select('.column-third')) == 3 + + assert len( page.select('.big-number-with-status {}'.format(big_number_class)) - ) + ) == 3 @freeze_time("2016-01-01 11:09:00.061258") From 220da2e5be7d9c3f38e034ea845209ff7b8add6a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 29 Oct 2019 16:27:58 +0000 Subject: [PATCH 4/4] Rename usage with letters template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s the only usage template now. --- app/main/views/dashboard.py | 2 +- app/templates/views/{usage-with-letters.html => usage.html} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename app/templates/views/{usage-with-letters.html => usage.html} (100%) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 8fadf10c6..49beb1a58 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -134,7 +134,7 @@ def usage(service_id): yearly_usage = billing_api_client.get_service_usage(service_id, year) return render_template( - 'views/usage-with-letters.html', + 'views/usage.html', months=list(get_free_paid_breakdown_for_billable_units( year, free_sms_allowance, diff --git a/app/templates/views/usage-with-letters.html b/app/templates/views/usage.html similarity index 100% rename from app/templates/views/usage-with-letters.html rename to app/templates/views/usage.html