From dcc590ec96fd1b3f45325e1b2330c0f5fda5e3d4 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 8 Jan 2019 17:52:01 +0000 Subject: [PATCH 1/4] Cancelled notifications do not show as failures on dashboard stats Also update tests after we no longer show cancelled letters --- app/templates/components/big-number.html | 21 ++++----------------- app/templates/views/dashboard/_totals.html | 3 +-- app/utils.py | 2 +- tests/app/main/views/test_activity.py | 4 ++-- tests/app/main/views/test_jobs.py | 5 ++--- tests/app/main/views/test_notifications.py | 4 ++++ 6 files changed, 14 insertions(+), 25 deletions(-) diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index c8314b2e1..8ca33b3fb 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -34,8 +34,7 @@ link=None, show_failures=True, smaller=False, - smallest=False, - failed_as_cancelled=False + smallest=False ) %}
{{ big_number(number, label, link=link, smaller=smaller, smallest=smallest) }} @@ -45,26 +44,14 @@ {% if failure_link %} {{ "{:,}".format(failures) }} - {% if failed_as_cancelled %} - cancelled - {% else %} - failed – {{ failure_percentage }}% - {% endif %} + failed – {{ failure_percentage }}% {% else %} {{ "{:,}".format(failures) }} - {% if failed_as_cancelled %} - cancelled - {% else %} - failed – {{ failure_percentage }}% - {% endif %} + failed – {{ failure_percentage }}% {% endif %} {% else %} - {% if failed_as_cancelled %} - 0 cancelled - {% else %} - No failures - {% endif %} + No failures {% endif %}
{% endif %} diff --git a/app/templates/views/dashboard/_totals.html b/app/templates/views/dashboard/_totals.html index bb6b01887..a7a9de599 100644 --- a/app/templates/views/dashboard/_totals.html +++ b/app/templates/views/dashboard/_totals.html @@ -37,8 +37,7 @@ 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, - failed_as_cancelled=True + smaller=smaller_font_size ) }} {% endif %} diff --git a/app/utils.py b/app/utils.py index 7e20d7680..d77b52f1a 100644 --- a/app/utils.py +++ b/app/utils.py @@ -40,7 +40,7 @@ from werkzeug.datastructures import MultiDict SENDING_STATUSES = ['created', 'pending', 'sending', 'pending-virus-check'] DELIVERED_STATUSES = ['delivered', 'sent', 'returned-letter'] FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', - 'technical-failure', 'virus-scan-failed', 'cancelled'] + 'technical-failure', 'virus-scan-failed'] REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 3e4fb7e10..929687d31 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -53,7 +53,7 @@ from tests.conftest import ( 'created', 'pending', 'sending', 'pending-virus-check', 'delivered', 'sent', 'returned-letter', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', - 'virus-scan-failed', 'cancelled', + 'virus-scan-failed', ] ), ( @@ -66,7 +66,7 @@ from tests.conftest import ( ), ( 'failed', - ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', 'cancelled'] + ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] ) ] ) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 22d5ac11a..b7121bed7 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -157,7 +157,7 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( 'created', 'pending', 'sending', 'pending-virus-check', 'delivered', 'sent', 'returned-letter', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', - 'virus-scan-failed', 'cancelled', + 'virus-scan-failed', ] ), ( @@ -170,7 +170,7 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( ), ( 'failed', - ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', 'cancelled'] + ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] ) ] ) @@ -340,7 +340,6 @@ def test_should_show_letter_job( 'permanent-failure', 'technical-failure', 'virus-scan-failed', - 'cancelled', ], ) diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index b9f3a93de..2e010dbc9 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -202,7 +202,11 @@ def test_notification_page_shows_page_for_letter_notification( ), ( 'validation-failed', +<<<<<<< HEAD 'Validation failed – content is outside the printable area', +======= + 'Can’t print this letter – content is outside the printable area.', +>>>>>>> Cancelled notifications do not show as failures on dashboard stats ), )) @freeze_time("2016-01-01 01:01") From 9f99ebd53b3475c383f11d97d34a9569d7f04ae0 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Jan 2019 11:34:47 +0000 Subject: [PATCH 2/4] notifications page title for failed letters is titled 'Failed letters' --- app/templates/views/notifications.html | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index efd758f3e..d6645f917 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -5,8 +5,13 @@ {% from "components/textbox.html" import textbox %} {% from "components/form.html" import form_wrapper %} +{% set title_status = ( + 'Failed ' + if status == 'failed' and message_type == 'letter' + else '' +) %} {% set page_title = ( - message_count_label(99, message_type, suffix='') | capitalize + (title_status + message_count_label(99, message_type, suffix='')) | capitalize if current_user.has_permissions('view_activity') else 'Sent messages' ) %} From 2dec697dc950f9fb7cd4daf614eb4f79750d8dbe Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 10 Jan 2019 15:43:38 +0000 Subject: [PATCH 3/4] Fix a test after merge conflict mixup --- tests/app/main/views/test_notifications.py | 23 +++++----------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 2e010dbc9..40125b565 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -191,24 +191,11 @@ def test_notification_page_shows_page_for_letter_notification( assert mock_page_count.call_args_list[0][1]['values'] == {'name': 'Jo'} -@pytest.mark.parametrize('notification_status, expected_message', ( - ( - 'permanent-failure', - 'Cancelled 1 January at 1:02am', - ), - ( - 'cancelled', - 'Cancelled 1 January at 1:02am', - ), - ( - 'validation-failed', -<<<<<<< HEAD - 'Validation failed – content is outside the printable area', -======= - 'Can’t print this letter – content is outside the printable area.', ->>>>>>> Cancelled notifications do not show as failures on dashboard stats - ), -)) +@pytest.mark.parametrize('notification_status, expected_message', [ + ('permanent-failure', 'Cancelled 1 January at 1:02am'), + ('cancelled', 'Cancelled 1 January at 1:02am'), + ('validation-failed', 'Validation failed – content is outside the printable area'), +]) @freeze_time("2016-01-01 01:01") def test_notification_page_shows_cancelled_letter( client_request, From bb1e9d2a0f183b8f987b5ef299ba189c0bfbc97e Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 10 Jan 2019 15:54:31 +0000 Subject: [PATCH 4/4] Add validation-failed status to FAILURE_STATUSES in admin utils --- app/utils.py | 2 +- tests/app/main/views/test_activity.py | 7 +++++-- tests/app/main/views/test_jobs.py | 8 ++++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/utils.py b/app/utils.py index d77b52f1a..6a7bb02e8 100644 --- a/app/utils.py +++ b/app/utils.py @@ -40,7 +40,7 @@ from werkzeug.datastructures import MultiDict SENDING_STATUSES = ['created', 'pending', 'sending', 'pending-virus-check'] DELIVERED_STATUSES = ['delivered', 'sent', 'returned-letter'] FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', - 'technical-failure', 'virus-scan-failed'] + 'technical-failure', 'virus-scan-failed', 'validation-failed'] REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 929687d31..a662b9996 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -53,7 +53,7 @@ from tests.conftest import ( 'created', 'pending', 'sending', 'pending-virus-check', 'delivered', 'sent', 'returned-letter', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', - 'virus-scan-failed', + 'virus-scan-failed', 'validation-failed' ] ), ( @@ -66,7 +66,10 @@ from tests.conftest import ( ), ( 'failed', - ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] + [ + 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', + 'virus-scan-failed', 'validation-failed' + ] ) ] ) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index b7121bed7..300dadb23 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -157,7 +157,7 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( 'created', 'pending', 'sending', 'pending-virus-check', 'delivered', 'sent', 'returned-letter', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', - 'virus-scan-failed', + 'virus-scan-failed', 'validation-failed' ] ), ( @@ -170,7 +170,10 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( ), ( 'failed', - ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] + [ + 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', + 'validation-failed' + ] ) ] ) @@ -340,6 +343,7 @@ def test_should_show_letter_job( 'permanent-failure', 'technical-failure', 'virus-scan-failed', + 'validation-failed' ], )